I want to sort a vector of dates and I wrote compare function for it:
#include <iostream>
struct date {
int day;
int month;
int year;
};
int compare_dates(date a, date b) {
if (a.year < b.year) {
return -1;
} else if (a.year == b.year) {
if (a.month < b.month) {
return -1;
} else if (a.month == b.month) {
if (a.day < b.day) {
return -1;
} else if (a.day > b.day) {
return 1;
}
} else {
return 1;
}
} else {
return 1;
}
return 0;
}
int main() {
date a = {};
date a.day = 19;
date a.month = 11;
date a.year = 2016;
date b = {};
date b.day = 20;
date b.month = 11;
date b.year = 2016;
compare_dates(a, b) // -1
compare_dates(b, a) // 1
compare_dates(b, b) // 0
return 0;
}
It is working well, but compare_dates function looks awful. Is there any idea how can I improve it?
I'm not a C++ expert and the others are pointing out that std::sort() doesn't require three-way comparison, only a <. But to clean up your code as written:
Your compare_dates() keeps doing three-way comparisons for >/</==, and wants a +1/-1/0 return value. So declare a three-way cmp() helper function which does that, like we do in Python. Now your code reduces to:
int cmp(int x, int y) {
return (x>y) ? 1 : ((x<y) ? -1 : 0);
}
int compare_dates(date a, date b) {
if (cmp(a.year, b.year) != 0)
return cmp(a.year, b.year);
if (cmp(a.month, b.month) != 0)
return cmp(a.month, b.month);
return cmp(a.day, b.day);
}
You only fall-through into doing the lower-order comparisons if the higher-order comparison gave '=='. So that allows you to avoid all the else-clauses, braces and indenting, which keeps your indent level constant and is easy on the eyes. It also calls out the symmetry of the computations.
This will be enough for sorting a containers of dates into ascending order:
bool compareDates(date const& lhs, date const& rhs) const {
if(lhs.year == rhs.year) {
if(lhs.month == rhs.month) {
return lhs.day < rhs.day;
}
return lhs.month < rhs.month;
}
return lhs.year < rhs.year;
}
// sort(dates, dates + n, compareDates);
Edit
I intentionally didn't handle -1 separately as for overriding comparator of STL containers like std::sort(), priority_queue or std::set we don't need to provide integer return code and make to code relatively complex. Boolean is enough.
What about using the fact that a day use only 4 bit and a month only 5?
#include <iostream>
struct date
{
int day;
int month;
int year;
};
int compare_dates (date a, date b)
{
long da { (a.year << 9) + (a.month << 4) + a.day };
long db { (b.year << 9) + (b.month << 4) + b.day };
return da < db ? -1 : (da > db);
}
int main()
{
date a = { 19, 11, 2016 };
date b = { 20, 11, 2016 };
std::cout << compare_dates(a, b) << std::endl; // print -1
std::cout << compare_dates(b, a) << std::endl; // print 1
std::cout << compare_dates(b, b) << std::endl; // print 0
return 0;
}
--- EDIT ---
As pointed by Christian Hackl, this code is a little obscure.
I hope that can be more comprensible if you translate the bitfield part in the date struct, trasforming it in a union.
So you can initialize separate year, month and day components and use a full component for compares.
Something as follows
#include <iostream>
union date
{
struct
{
unsigned long day : 5U;
unsigned long month : 4U;
unsigned long year : 23U;
} s ;
unsigned long full;
};
int compare_dates (date const & a, date const & b)
{ return a.full < b.full ? -1 : (a.full > b.full); }
int main()
{
date a = { { 19, 11, 2016 } };
date b = { { 20, 11, 2016 } };
std::cout << compare_dates(a, b) << std::endl; // print -1
std::cout << compare_dates(b, a) << std::endl; // print 1
std::cout << compare_dates(b, b) << std::endl; // print 0
return 0;
}
