FYI: Patch: java.text.SimpleDateFormat merge from classpath
Bryce McKinlay
bryce@mckinlay.net.nz
Sat Sep 27 23:04:00 GMT 2003
On Sunday, Sep 28, 2003, at 02:01 Pacific/Auckland, Guilhem Lavaux
wrote:
> >> +2003-09-25 Guilhem Lavaux <guilhem@kaffe.org>
> >> +
> >> + * java/text/SimpleDateFormat.java (parse): Don't use class
> calendar
> >> + field.
> >
> >
> > This patch doesn't look right. We don't want to be cloning the
> calendar object on every parse() call - its a big performance penalty.
> If calendar.clear() is not correctly reseting the calendar instance,
> then the bug is in Calendar, not SimpleDateFormat.
> >
> > Guilhem, could you explain the motivation behind this change?
> >
> > Regards
> >
> > Bryce
> >
> Hmm... apparently I should only have done a "new Calendar". I am
> clearing it just after.
> The problem was that if you try to parse a string using
> SimpleDateFormat and you don't create an isolated calendar object you
> may contamine the old one. But the old calendar may be used to format
> new strings and the mauve tests was doing precisely that. The results
> were obviously different as parse used to modify calendar. But I admit
> I should have only done a "new GregorianCalendar".
I'm still not convinced thats the right solution - "new" is no better
than clone(). The calendar object is supposed to be reused, as the call
to calendar.clear() should reset any state held in it. Thus it should
not be "contaminated" by previous calls. Can you give me a pointer to
the failing mauve test?
Regards
Bryce
More information about the Java-patches
mailing list