On 15/02/2008, Emmanuel Bourg <[EMAIL PROTECTED]> wrote: > sebb a écrit : > > > > The static DateFormat is not private, so the synchronization is not > guaranteed. > > > I'll make it private. I added a comment mentioning that it must be > synchronized. > > > > > Likewise the instance DateFormat. > > > > Indeed that is worse, since the format is temporarily changed by the > > private method - even if the field is made private, the method(s) that > > use it are potentially thread-hostile. > > > > Seems to me that the DateFormats should both be private. > > > I left the instance DateFormat package private only to access it from > the test cases. I'll synchronize its use as well. >
The instance field ought to have a comment to say why it is package protected, and to point out that any multi-threaded use must be synchronized on DATE_FORMAT. The testcase may need to be updated accordingly. Should probably add an @GuardedBy() annotation as well. > > > >> The package name has changed, is it still necessary to change the UID ? > >> > > > > As far as I can tell the package name was not changed in this update, > > but if the previous version of the class was never deployed then it's > > probably not necessary. > > > Indeed, it's a newly created experimental branch. > > > > >> FastDateFormat could be useful to simplify the code on the 1.x branch, > >> but for the 2.x code base I think SimpleDateFormat is good enough. > > > > I don't follow that reasoning. > > > On the 1.x branch we were parsing a timezone with a SimpleDateFormat, > since this was not supported by Java 1.3 Oliver implemented an > alternative date parser. FastDateFormat could probably replace this > implementation since it supports the timezone and works on Java 1.3. > > On the 2.x branch Java 5 is the minimum requirement, so the Java 1.3 > compatibility of FastDateFormat is not a compelling reason to adopt it. > >From the Javadoc: "FastDateFormat is a fast and thread-safe version of SimpleDateFormat." This is why I suggested using it, not because it is 1.3 compatible. > > Emmanuel Bourg > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]