4 ms·
> Line #263: if (days > 366) This should work if that was a >= sign. And people at work don't believe me when I say how insanely careful I have to be when I
by chime 18y ago
> Line #263: if (days > 366)
This should work if that was a >= sign. And people at work don't believe me when I say how insanely careful I have to be when I write any code, especially code that deals with people's payroll and vacation time. One missing equals symbol and every Zune in the world froze. One missing equals symbol and every employee could lose 1/12 of their accumulated vacation time.
The ability to pay attention to "mundane details" is just as important as the ability to envision the whole software application from the project manager point-of-view, especially when working in smaller companies without additional oversight.
- deleted 18y ago[deleted]
- tlrobinson 18y agoMaking 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; }
- judofyr 18y agoLet's be a little more DRYer: days_in_year = IsLeapYear(year) ? 366 : 365; while (days > days_in_year) { days -= days_in_year; year += 1; }
- blablebla 18y agoHow about a little more CORRECTer? #define diy(y) (IsLeapYear(y) ? 366 : 355) while (days > diy(year)) { days -= diy(year); year += 1; }
- ars 18y agoFor 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; }
- tlrobinson 18y agoIncorrect, 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; }
- jabr 18y agoI prefer: for (;;) { int days_in_year = IsLeapYear(year) ? 366 : 365; if (days <= days_in_year) break; days -= days_in_year; year += 1; }
- thras 18y agoNo. That's incorrect. That would cause the year to go up on Dec. 31st (day 366), turning it into day 0. In fact, what you want is to change the while to an if statement. If it really does need to run as a while statement (to add more than one year at once?), then the alternative would be a break statement after line 271.
- Jebdm 18y agoReally, this looks like a rather silly way to put this function together. It doesn't even run in constant time! It might be a little complicated, due to the leap year rules, but it would probably not be too difficult to do this calculation without any loops.
- jrockway 18y ago> One missing equals symbol and every Zune in the world froze. Wrong. Completely ignoring all software engineering "best practices" was the problem. They could have reused a working library. They could have written unit tests tests. QA could have tested it. Other developers could have reviewed it, or written test cases without knowledge of how the code worked. Basically, they ignored every rule for writing good code, and then ended up with a serious bug. Well, yeah. The whole "unit testing" "fad" exists for a reason -- it greatly improves the quality of software. Many people take software development seriously enough to not let one brain fart cause a piece of software to brick thousands of devices.