When you have exceptions, who needs return?
-
I sent this to Alex a few months ago but he didn't post it, probably
because it's sample code or he has too many others, so I thought I'd
share it with y'all.
I found a bug-hunting article on InformIT the other day that asks us to
find a minor semantic bug in one of the worst-designed functions I've
ever seen.
1. public class IsLeapYear {
2.
3. public static class LeapYearException
4. extends Exception {}
5. public static class NotLeapYearException
6. extends Exception {}
7.
8. static void checkLeapYear(String year)
9. throws LeapYearException, NotLeapYearException,
10. NumberFormatException {
11.
12. long yearAsLong = Long.parseLong(year);
13.
14. //
15. // A leap year is a multiple of 4, unless it is
16. // a multiple of 100, unless it is a multiple of
17. // 400.
18. //
19. // We calculate the three values, then make a
20. // 3-bit binary value out of them and look it up
21. // in results.
22. //
23.
24. final boolean results[] =
25. { true, false, false, true,
26. false, false, false, false };
27.
28. if (results[
29. ((((yearAsLong % 4) == 0) ? 1 : 0) << 2) +
30. ((((yearAsLong % 100) == 0) ? 1 : 0) << 1) +
31. ((((yearAsLong % 400) == 0) ? 1 : 0) << 0)]) {
32. throw new LeapYearException();
33. } else {
34. throw new NotLeapYearException();
35. }
36. }
37.
38. public static void main(String[] args) {
39.
40. if (args.length > 0) {
41.
42. try {
43. checkLeapYear(args[0]);
44. } catch ( NumberFormatException nfe ) {
45. System.out.println(
46. "Invalid argument: " +
47. nfe.getMessage());
48. } catch ( LeapYearException lye ) {
49. System.out.println(
50. args[0] + " is a leap year");
51. } catch ( NotLeapYearException nlye ) {
52. System.out.println(
53. args[0] + " is not a leap year");
54. }
55. }
56. }
57. }Whoa, that was awesome, Firefox pasted it in formatted and everything. Cool.
The error is that the table lookup is reversed, and gives the wrong
answer every time. The real bug... worst use of exceptions, worst use
of bitshifts, and all this complexity for the equivelent of a one-line
C macro. Can you say "just because I could"?
return (year % 4
== 0 && year % 100 != 0 && year % 400 == 0); does the
job quite nicely in my experience. (You'll want to test that it's a number and not null first.) Or Date.isLeapYear().
-
@foxyshadis said:
return (year % 4
== 0 && year % 100 != 0 && year % 400 == 0); does the
job quite nicely in my experience. (You'll want to test that it's a number and not null first.) Or Date.isLeapYear().
Let year = 4
year % 4 == 0 (true)
year % 100 != 0 (true)
year % 400 == 4 != 0 (false)
Let year = 400
year % 4 == 0 (true)
year % 100 == 0 (false)
year % 400 == 0 (true)
Well, at least it doesn't get any false positives.
This code might work a bit better:
[code language="c#"]return (year % 400 == 0) || (year % 4 == 0 && year % 100 != 0);[/code]
-
Hey that's a pretty good one; not sure how that one didn't get posted. I'm a bit disorganized ...
-
Oops, I meant an or in there.
I'd hate to be the guy trying to figure out why calling this function always crashes my program with an unhandled exception.
-
Well, at least it's y2k compliant:
12. long yearAsLong = Long.parseLong(year);
Guaranteed to work to year 2147483648 and beyond...
-
@foxyshadis said:
24. final boolean results[] =
25. { true, false, false, true,
26. false, false, false, false };
27.
28. if (results[
29. ((((yearAsLong % 4) == 0) ? 1 : 0) << 2) +
30. ((((yearAsLong % 100) == 0) ? 1 : 0) << 1) +
31. ((((yearAsLong % 400) == 0) ? 1 : 0) << 0)]) {It's quite funny to note that in addition to all the other problems with this code (mentioned by others and the original poster), it's a horribly inefficient use of the array. All things divisible by 100 are divisible by 4, all by 400 by 100 and 4, etc., so that it's not even possible to look at the indexes (001, 010, 011, 101), so that only four of the 8 entries in the array can ever be indexed...
Really a smart one, this guy.
Find the Bug in this Java Program | Brief Summary of Java | InformIT
