0

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?

smci
  • 32,567
  • 20
  • 113
  • 146
Dmytro Rudnitskikh
  • 111
  • 10

3 Answers3

3

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.

Kaidul
  • 15,409
  • 15
  • 81
  • 150
  • No, `lhs.day < rhs.day` wil only evaluate to `0:False` or `1:True`. This will never return -1. It will return 0 for both `==` and `>` cases. – smci Nov 19 '16 at 12:36
  • 0 for `==` is desirable to prevent unnecessary swapping on equality, and ensure termination (/early-termination) of sorts. – smci Nov 19 '16 at 12:44
  • 2
    @smci "This will never return -1." Not needed - e.g the [`std::set`](http://en.cppreference.com/w/cpp/container/set) (which is a sorted set) does it only with `std::less`. Even if you'd need 3-ways comparison and you only have `<`, you can still do `if (a – Adrian Colomitchi Nov 19 '16 at 13:00
  • @smci Yes, you're right. But think twice before down-voting. Here we don't need -1 for overriding comparator. `0 for == is desirable to prevent unnecessary swapping on equality` - there is no mention to do stable sort. – Kaidul Nov 19 '16 at 14:12
  • Yes @KaidulIslam I acknowledged you're correct and unfortunately the downvote is locked in by SO. I thought all high-level languages these days considered a builtin three-way comparison to be better. – smci Nov 19 '16 at 14:19
  • @smci It's okay. I've edited my answer. Now you will be able to do. – Kaidul Nov 19 '16 at 14:24
  • Done. FYI I had read the doc on `std::pair` and other parts of `std` and did think twice. – smci Nov 19 '16 at 14:37
  • @smci No offense. I am a Java guy and know how common the three-way comparison is. Lets up-vote your answer. Cheers! – Kaidul Nov 19 '16 at 14:49
3

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.

smci
  • 32,567
  • 20
  • 113
  • 146
  • I think we can improve your answer saving `cmp(a.year, b.year)` and `cmp(a.month, b.month)` in additional variables. – Dmytro Rudnitskikh Nov 19 '16 at 12:47
  • @DmytroRudnitskikh: No, compilers these days do obvious optimizations like that. – smci Nov 19 '16 at 12:48
  • hm, actually I am JavaScript developer. Do you know V8 engine do the same optimization? What about maintainability of this approach? E.g. if `cmp` function changes a name, we will need to do change it in twice as much more places. – Dmytro Rudnitskikh Nov 19 '16 at 12:57
  • Sorry, can't say, I was expecting there'd be a C++ builtin or library function for three-way compare; I browsed [tag:c++] questions but couldn't find out. This is not a definitive answer, but I was just showing you how three-way cmp function and only falling-through on the `0:==` case makes your coding life easier. As to JS, I don't know about JS engine optimizations... – smci Nov 19 '16 at 13:01
  • @DmytroRudnitskikh: I suspect the C++ idiom is to declare it as a class rather than as a struct, and declare either a `sort()` function or at least the minimal operators needed for sorting from `operator== operator!= operator< operator<= operator> operator>=` . Please see other C++ questions on sorting of custom objects. – smci Nov 19 '16 at 13:08
  • 1
    @smci: The C++ idiom is to either provide `operator<` (the others are not needed because they can be deduced from `<`) or to write a custom functor for when you actually `std::sort`. `struct` or `class` is a different question altogether and not related to comparison. 3-way compares are either C-isms or Java-isms. – Christian Hackl Nov 19 '16 at 13:51
  • @ChristianHackl: most high-level languages Python, Ruby, PERL, PHP also do [Three-way comparisons](https://en.wikipedia.org/wiki/Three-way_comparison). I should have said that yeah you(/the compiler) can always synthesize a three-way comparison from a '<' by doing two comparisons `(xy)` but that may incur extra operations. For a struct like this, the three-way result is essentially for free because it's same effort as a simple '<' – smci Nov 19 '16 at 14:06
  • @smci After downvoting my answer for not handling -1, now you're editing your question `I'm not a C++ expert and the others are pointing out that std::sort() doesn't require three-way comparison, only a <.` – Kaidul Nov 19 '16 at 14:14
  • 1
    @smci: Yes, other languages also have three-way comparisons. I think C++ may even be the only high-level language which actually came up with something better than C here. Note that using `std::tie`, available since C++11, the standard solution is also much easier to write, see http://en.cppreference.com/w/cpp/utility/tuple/tie – Christian Hackl Nov 19 '16 at 14:17
  • @KaidulIslam: I already tried to remove the downvote after you C++ folks corrected me, but SO locks in downvotes. I acknowledged that the three-way comparison is probably unnecessary and that you guys corrected me. What more can I do? I do not game SO for points. – smci Nov 19 '16 at 14:17
  • 1
    Why do you do check `xy`? – David G Nov 19 '16 at 16:46
  • @0x499602d2: The OP was asking how to implement their three-way comparison function, not how to implement sort. To implement their three-way comparison using only '<' would require 2 comparisons in the source-code which if not optimized by compiler would cause an expected 1.5 comparisons at runtime. I guess the compiler would merge the two comparisons, i.e. synthesize the `cmp()` or `<=>` comparison. – smci Nov 19 '16 at 16:49
  • I don't think you actually read @0x499602D2's question; `return (x>y) ? ( (xy then check if xy ? 1 : x – ildjarn Nov 19 '16 at 23:00
  • @ildjarn: oh sorry thanks for the correction both of you – smci Nov 19 '16 at 23:48
0

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;
 }
max66
  • 65,235
  • 10
  • 71
  • 111