If You Don't Test Boundary Conditions...

Comments

January 1, 2009 at 9:45 pm

(1) mike says:

…
year = ORIGINYEAR; /* = 1980 */

{
while (days > 365)

if (IsLeapYear(year))
{
if (days > 366)
{
days -= 366;
year += 1;
}
}
else
{
days -= 365;
year += 1;
}
}

is it right now?

January 2, 2009 at 4:57 am

(2) nolan de souza says:

David, when the year is leap and days is 366, the while loops forever. The inner if should have had a else clause to break out of the while loop for this day, so that normal processing would continue.

if (days > 366)
{
days -= 366;
year += 1;
}
else
{
break;
}

January 5, 2009 at 9:58 am

(3) KRW says:

The primary coding issue here is the use of literal constants 365 and 366. These should be replaced with a variable named something like daysInThisYear which would be set to 365 or 366 based on IsLeapYear returning true/false. Other code changes would be required of course, but the flaw is in the assumption made about the value 365 in the while loop logic. Simply putting a break statement in, as nolan suggests, will cause the code to break out at the first leap year you encounter (e.g. days value could be 1461…I think) you would have a different problem. Not following mike’s change…looks like he just moved a brace?

January 5, 2009 at 10:23 am

(4) KRW says:

Change that 1461 to 1827…forgot starting from 1980…

January 5, 2009 at 10:33 am

(5) KRW says:

OK, Nolan is correct…now that I’ve actually run the code, but I still believe it is troublesome to use hard-coded values in general, but particularly wrong when the value in question is truly a variable and not a constant.