Re: [cp-patches] GregorianCalendar mess

[Bryce McKinlay]

Jeroen Frijters wrote:

> > Perhaps a separate pass to normalize the fields is needed,
> > which can be called by both calculateDay() and computeTime()?
> >    
>
> The nasty thing about this class is that all of its state is protected,
> so subclasses could (theoretically) depend on all sorts of nasty
> implementation details. I don't know if we should care about such a
> theoretical scenario, but it bothers me never the less.
>  

Yeah, subclasses that (ab)use the protected state are asking for 
trouble. I think its much more important to get it right in terms of the 
public interfaces than worry about what subclasses might do.

> That doesn't work. The docs explicitly say that set doesn't do anything
> other than setting that particular field and the result of roll over
> depends on the contents of the other fields. For example, the follow
> code should print two identical dates:
>
> GregorianCalendar cal = new GregorianCalendar();
>
> cal.set(Calendar.MONTH, Calendar.FEBRUARY);
> cal.set(Calendar.DAY_OF_MONTH, 31);
> cal.set(Calendar.MONTH, Calendar.MARCH);
> System.out.println(cal.getTime());
>
> cal.set(Calendar.MONTH, Calendar.MARCH);
> cal.set(Calendar.DAY_OF_MONTH, 31);
> cal.set(Calendar.MONTH, Calendar.MARCH);
> System.out.println(cal.getTime());
>  

On the other hand, consider something like:

cal.set(Calendar.MONTH, Calendar.FEBRUARY);
cal.set(Calendar.DAY_OF_MONTH, 31);
System.out.println(cal.get(Calendar.DAY_OF_MONTH));



This will print "2" (in a leap year), indicating that the fields are 
being rolled/recalculated upon any get() call.

I still think the right thing to do is to have a separate 
recalculation/normalization pass, including processing any roll-overs 
that might be required due to out-of-range values in a lenient instance. 
I think that proper rolling behaviour (currently we are pretty buggy 
here) could be implemented relatively easily using recursion. For 
example, if someone has set MINUTES to an out of range value, 
recalculate() updates the minute field, then since it rolls over to the 
hours field, call recalculate(Calendar.HOUR_OF_DAY, ...) with the 
recalculated hours value, which will may turn call 
recalculate(Calendar.DAY_OF_YEAR,..), and so on. Note that this 
recalculate() method would be different from the current 
calculateFields(), because it recalculates the fields solely based on 
the values of other fields, not from the milliseconds time value.

This recalculation should be done, if necessary, at the start of any of 
the get() calls. This should fix numerous bugs with the current 
implementation, and would simplify other methods such as calculateTime() 
as they will no longer have to deal with out-of-range values.

An interesting observation from mucking around with test cases: at least 
in JDK 1.5, isSet() always returns true.

Regards

Bryce