commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r628021 - in /commons/proper/configuration/branches/configuration2_experimental: src/main/java/org/apache/commons/configuration2/plist/ src/test/java/org/apache/commons/configuration2/plist/ xdocs/
Date Fri, 15 Feb 2008 16:55:22 GMT
On 15/02/2008, Emmanuel Bourg <ebourg@apache.org> 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: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message