How can I optimize compare function for custom dat

Question

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?

Answer 1

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.

Answer 2

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.

Answer 3

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;
 }