tags:

views:

178

answers:

3

I'm reading Uncle Bob's "The Craftsman" series, and have gotten to #29 (PDF). In it, there's this snippet in test code, for asserting dates are close enough:

private boolean DatesAreVeryClose(Date date1, Date date2) {
  GregorianCalendar c1 = new GregorianCalendar();
  GregorianCalendar c2 = new GregorianCalendar();
  c1.setGregorianChange(date1);
  c2.setGregorianChange(date2);
  long differenceInMS = c1.getTimeInMillis() - c2.getTimeInMillis();
  return Math.abs(differenceInMS) <= 1;
}

I read the docs, and yet couldn't figure why simply using Date.getTime() isn't good enough instead of introducing the calendar etc. Am I missing some corner cases?

A: 

There is no good reason not to use Date.getTime(). Most likely, the author was falling back on his familiar use of the GregorianCalendar object for doing date/time manipulation.

On a side note, use of setGregorianChange to get this information seems... abusive of the API. The method is used to set "the point when the switch from Julian dates to Gregorian dates occurred. Default is October 15, 1582 (Gregorian). " (javadoc) It may work to make the given calculation, but it makes things pretty hard to understand from a code maintenance point of view.

Alex B
+1  A: 

Time on the spaceship is different from time in our world. Also, the java API on the spaceship is slightly different from the API in our world. The method setGrogorianChange has nothing to do with julian dates in the spaceship world. Rather it is a way to set the time of the GregorianCalendar instance. And in the spaceship world, Date.getTime() does not return milliseconds. It returns a hashed value that somehow represents the time, but cannot be subtracted or added.

So there.

Robert C. Martin
This answer makes about as much sense as the code in the question and the linked pdf, on a spaceship.
Wouter Coekaerts
Well, given that he wrote them both, I think it's his right.
abyx
+1  A: 

I'm going to bite: that code is utter rubbish. If the use of setGregorianChange() has any purpose at all (which I doubt), it is the kind of clever hack that has no business anywhere near real world code. But I strongly suspect that whoever wrote that cutesy story just didn't know the Date/Calendar API very well and really meant to use Calendar.setTime().

The case against directly comparing Date instances in assertEquals() seems to be that the instances' internal timestamps may differ by milliseconds, even if created right after on another - thus the introduction of a "fuzzy compare" in the form of Math.abs(differenceInMS) <= 1. But neither does that require the detour via GregorianCalendar, nor is it enough fuzz - a full GC or even just the clock granularity could easily lead to two dates created right after another being 10 or more ms apart.

The real problem is the lack of a "calendar date" datatype in Java - making comparisons with day granularity pretty verbose (you have to use a Calendar to set all time fields to 0). Joda Time's DateMidnight class is what is really needed here.

Michael Borgwardt