Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Making that ">=" would make that entire "if" check redundant, since we already know it's at least 365 (because of the "while" condition)

I think the "while" condition should actually be

    while ((days > 365 && !IsLeapYear(year)) || (days > 366 && IsLeapYear(year)))
(or perhaps some simplification that I'm too lazy to figure out right now)

And then the inner "if" condition can be eliminated. Thus the final code could be:

    while ((days > 365 && !IsLeapYear(year)) || (days > 366 && IsLeapYear(year)))
    {
        if (IsLeapYear(year))
            days -= 366;
        else
            days -= 365;
        year += 1;
    }
This seems logical and easier to understand than the original too.

Edit: if you want to get ternary-operator fancy:

    while (days > (IsLeapYear(year) ? 366 : 365))
    {
        days -= IsLeapYear(year) ? 366 : 365;
        year += 1;
    }


Let's be a little more DRYer:

    days_in_year = IsLeapYear(year) ? 366 : 365;
    while (days > days_in_year) {
        days -= days_in_year;
        year += 1;
    }


How about a little more CORRECTer?

#define diy(y) (IsLeapYear(y) ? 366 : 355)

while (days > diy(year)) { days -= diy(year); year += 1; }


For the confused: the year changes each time, so you need to recheck IsLeapYear each time.

But I wouldn't do it with a define but rather:

while (days > (days_in_year = IsLeapYear(year) ? 366 : 365)) { days -= days_in_year; year += 1; }


Incorrect, since you don't update days_in_year on each loop iteration. If you really want...

    while (days > (days_in_year = IsLeapYear(year) ? 366 : 365)) {
        days -= days_in_year;
        year += 1;
    }


I prefer:

  for (;;) {
    int days_in_year = IsLeapYear(year) ? 366 : 365;
    if (days <= days_in_year) break;
    days -= days_in_year;
    year += 1;
  }




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: