Cool.
Suggestions for improvements are welcome. OK, you asked for it.
Inside getInstance(), I think
return new TimeSpan(value);
should be
return new TimeSpan(time);
Other stuff:
It looks like someone's using a tool which imports javadoc comments from overridden methods. Unfortunately they aren't always correct for the new class. (E.g. the toString() method.) And even if they're correct, they're often dead weight - there's no need to repeat the general contract of equals() and hashCode(); javadoc will provide a link to the overridden methods anyway. In general, the current comments tell you nothing that's not apparent from reading the method names. My feeling is that when that's the case, they might as well be deleted for readability, or replaced.
The javadoc should probably explain the difference between something like getDays() and getTotalDays(). And provide some usage examples, since some of it's not immediately apparent.
It seems that sometimes a TimeSpan represents a difference in times, and sometimes it represents an absolute time. Clarification might be appropriate.
As long as time can be a long, I'd modify the add(int, int) and subtract(int, int) methods to use longs as well. Also might be nice to check for overflow - though once you're using longs the incidence of that should be a lot rarer.
(And I admit it will be pretty rare for ints as well.)
And wouldn't
be easier to maintain?
Or for that matter, helper method like
would simplify 3 other methods, and add some useful validation.
Alternately the conversion factors could be put into a nice array, eliminating the switch entirely. Or, heck, the int values for DAYS, HOURS, etc. could
be the conversion factors. Though that's probably a bit more confusing to others reading the code.
Is there a reason to have getInstance(int, int) as a factory method, and also public constructors TimeSpan(long) and TimeSpan(Date)? Seems like the API would be cleaner if either they were all three constructors, or all three factory methods (with a single private constructor elsewhere).
It's tempting to add units of WEEKS and YEARS, too. I admit YEARS is a bit of a problem though since the desired conversion factor is not obvious. (Which is why I don't mention MONTHS here.) Perhaps NONLEAP_YEARS (365 days), LEAP_YEARS (366), and STANDARD_YEARS (365.24...)? OK no one would have a good reason to make a TimeSpan of more than one consecutive leap year. :roll: But offering the option encourages people to decide what they really mean, which I think is good.
[ January 28, 2003: Message edited by: Jim Yingst ]