You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Sean Schofield <ne...@schof.com> on 2003/01/07 22:33:30 UTC
[lang] FastDateFormat Update
FastDateFormat has finally been added to CVS. As mentioned in earlier
emails, this source code was originally written by Brian O'Neill and
reused/modified with his permission. This email contains a summary of
the changes made as well as issues to be resolved.
Changes already made:
=====================
1) I removed two of the overloaded getInstance() methods that took
DateFormatSymbols as arguments. SimpleDateFormat does allow a
DateFormat to be constructed using DateFormatSymbols but I thought for
now it might be nice to leave this out. It could be argued, however,
that we should include support for this, since some have described this
class as a thread-safe replacement for SimpleDateformat. (Actually the
changes related to this were commented-out for now so we can undo them
if people want.)
2) Removed 'throws IllegalArgumentException' from several methods. IMO,
runtime exceptions should not be declared in the method. I checked
several other classes in commons-lang and it seems that this follows the
current commons-lang coding convention as well.
3) Changed the "style" argument in getDateInstance() and
getTimeInstance() from Object to int since the method was converting
everything to int anyways. (Forgot to do this for getDateTimeInstance()
but this will suffice for now as I'm not even sure we want to keep these
methods in this class [see next section].)
4) Changed Brian's Pair.java to be a private static inner class of
FastDateFormat.
Proposed/Anticipated Changes
============================
1) Clean up source code (including javadoc) to conform to commons-lang
style. So fart this has been done with my changes only in an attempt to
get things checked in and get some feedback. The code should be cleaned
up as we polish this (and the test class.)
2) Eliminate getDateInstance(), getDateTimeInstance(), and
getTimeInstance() from FastDateFormat. I think the goal behind these
methods is fine but I don't think they belong in this class which is
supposed to be a light-weight and thread-safe SimpleDateFormat
replacement. Earlier we had discussed the ability to have some common
date/time formats available in DateUtils. This is where I propose this
functionality be moved to.
3) If we don't eliminate the methods in proposal #2, then we need to
investigate them for thread-saftey. They are all using SimpleDateFormat
methods and its been mentioned that some of these methods are not thread
safe (not sure if there is a problem with the specific ones used here.)
4) Consider replacing use of HashMap due to the problem that Dick has
mentioned.
Any thoughts?
- sean
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Stephen Colebourne <sc...@btopenworld.com>.
From: "Henri Yandell" <ba...@generationjava.com>
> Any particular reason not to use a synchronised Map?
>
> I guess it's the trade-off between the speed of a Hashtable and the
> nicer-API of a Map.
Hashtable implements Map ;-)
Stephen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Henri Yandell <ba...@generationjava.com>.
On Tue, 7 Jan 2003, Sean Schofield wrote:
> Thanks for the feedback. FastDateFormat doesn't seem to be as thread
> safe as advertised (at least in the version we are using.) There are a
> few instances of HashMap (some of which I was already proposing to
> eliminate b/c they are only being used by the getDateTimeInstance, etc.
> methods).
>
> I looked at the FastHashMap as a replacement and I have some major
> misgivings given the dire javadoc warnings and the equally pessimistic
> article they reference. I don't see a problem with using Hashtable.
Any particular reason not to use a synchronised Map?
I guess it's the trade-off between the speed of a Hashtable and the
nicer-API of a Map.
Hen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Sean Schofield <ne...@schof.com>.
Thanks for the feedback. FastDateFormat doesn't seem to be as thread
safe as advertised (at least in the version we are using.) There are a
few instances of HashMap (some of which I was already proposing to
eliminate b/c they are only being used by the getDateTimeInstance, etc.
methods).
I looked at the FastHashMap as a replacement and I have some major
misgivings given the dire javadoc warnings and the equally pessimistic
article they reference. I don't see a problem with using Hashtable.
- sean
PS. See below for Dick's comments
>>4) Consider replacing use of HashMap due to the problem that Dick has
>>mentioned.
>>
>>
>
>Damn. Now you make me have to pay attention to 2 emails at once. No clue
>which one was this though.
>
>
>
<snip>
Hi,
I saw your posting for the FastDateFormat class and noticed that you have
removed the synchronization on the getInstance method.
Shouldn't you then protect the call to cInstanceCache.put by doing a clone
of the Hashmap first (similar to the commons collections FastHashMap class)
to make sure that this works correctly in a multi-threaded environment?
Best regards,
Dick Zetterberg
</snip>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Sean Schofield <ne...@schof.com>.
>
>
>Any reason why we can't just delegate the parse() method to the
>SimpleDateFormat?
>
>
Wouldn't there be some thread-safety issues there? If not, then this is
a possibility.
>If we don't extend Format, I think it should be called FastDateFormatter.
>
>
How about we call it FastDateFormatter? Then if we get ambitious
enough, we add the rest of the stuff to make it truly extend DateFormat
(and rename it back to FastDateFormat).
I wonder if FastDateFormatter is useful enough if you couldn't create
Dates with it? I maintain it is, because there are several times where
I have the system date/time and need to format it. What do others think?
- sean
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Henri Yandell <ba...@generationjava.com>.
On Wed, 8 Jan 2003, Sean Schofield wrote:
> >Yup. Definitely _has_ to extend Format. Even if it isn't seen as a
> >SimpleDateFormat option.
> >
> So are you saying it should extend Format? Why should this be required?
> And if we do want to extend Format, what do you we do about the parsing
> requirements. The original FastDateFormat supported formatting only (no
> parsing from text to Date). We could take a stab at adding that part
> but I'd probably need some help there...
>
If it ends with 'Format', then I think it's pretty commonly understood to
be an extension of java.text.Format. Just as if it ended with Map, or
Iterator.
Something I do/aim-to-do with Format objects is to have a Formatter class
which wraps a ClassMap of Class to Format. Then I'd say:
formatter.format(date)
and it would look up java.util.Date, see it maps to FastDateFormat and use
that.
Any reason why we can't just delegate the parse() method to the
SimpleDateFormat?
If we don't extend Format, I think it should be called FastDateFormatter.
> - sean
>
> ps. Should we start using [lang][time] in the subject lines to spare
> people the volume of emails dedicated to the time subpackage? Or is
> there an exisiting commons-dev mailing list convention for this?
It's not that noisy yet :) If it did get noisy, [and it would have to be a
lot noisier] then I imagine we'd have to go for a Lang mailing list or
something. Which would probably be a bad idea as the Commons community in
general often have a lot to say on Lang.
Hen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Sean Schofield <ne...@schof.com>.
>
>
>On Tue, 7 Jan 2003, Stephen Colebourne wrote:
>
>
>>----- Original Message -----
>>From: "Henri Yandell" <ba...@generationjava.com>
>>
>>
>>>On Tue, 7 Jan 2003, Sean Schofield wrote:
>>>
>>>
>>>>1) I removed two of the overloaded getInstance() methods that took
>>>>DateFormatSymbols as arguments. SimpleDateFormat does allow a
>>>>DateFormat to be constructed using DateFormatSymbols but I thought for
>>>>now it might be nice to leave this out. It could be argued, however,
>>>>that we should include support for this, since some have described this
>>>>class as a thread-safe replacement for SimpleDateformat. (Actually the
>>>>changes related to this were commented-out for now so we can undo them
>>>>if people want.)
>>>>
>>>>
>>>If there's no reason as to why they're a pain to keep in, would be nice to
>>>not get some javadoc-lawyer hitting us with the reason why FastDateFormat
>>>isn't a SimpleDateFormat replacement.
>>>
>>>
>>Its not a SimpleDateFormat drop-in as it doesn't extend Format. It should do
>>(not the parsing, just the formatting part - throw
>>UnsupportedOperationException from the parsing)
>>
>>
>
>Yup. Definitely _has_ to extend Format. Even if it isn't seen as a
>SimpleDateFormat option.
>
>
So are you saying it should extend Format? Why should this be required?
And if we do want to extend Format, what do you we do about the parsing
requirements. The original FastDateFormat supported formatting only (no
parsing from text to Date). We could take a stab at adding that part
but I'd probably need some help there...
- sean
ps. Should we start using [lang][time] in the subject lines to spare
people the volume of emails dedicated to the time subpackage? Or is
there an exisiting commons-dev mailing list convention for this?
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Henri Yandell <ba...@generationjava.com>.
On Tue, 7 Jan 2003, Stephen Colebourne wrote:
> ----- Original Message -----
> From: "Henri Yandell" <ba...@generationjava.com>
> > On Tue, 7 Jan 2003, Sean Schofield wrote:
> > > 1) I removed two of the overloaded getInstance() methods that took
> > > DateFormatSymbols as arguments. SimpleDateFormat does allow a
> > > DateFormat to be constructed using DateFormatSymbols but I thought for
> > > now it might be nice to leave this out. It could be argued, however,
> > > that we should include support for this, since some have described this
> > > class as a thread-safe replacement for SimpleDateformat. (Actually the
> > > changes related to this were commented-out for now so we can undo them
> > > if people want.)
> >
> > If there's no reason as to why they're a pain to keep in, would be nice to
> > not get some javadoc-lawyer hitting us with the reason why FastDateFormat
> > isn't a SimpleDateFormat replacement.
>
> Its not a SimpleDateFormat drop-in as it doesn't extend Format. It should do
> (not the parsing, just the formatting part - throw
> UnsupportedOperationException from the parsing)
Yup. Definitely _has_ to extend Format. Even if it isn't seen as a
SimpleDateFormat option.
> > > 2) Removed 'throws IllegalArgumentException' from several methods. IMO,
> > > runtime exceptions should not be declared in the method. I checked
> > > several other classes in commons-lang and it seems that this follows the
> > > current commons-lang coding convention as well.
> >
> > I think this is quite debatable and up-in-the-air. You're not shattering
> > any rules by not having them in there, but there's a definite school of
> > thought in favour of them being there.
>
> I oppose putting runtime exceptions in the throws clause, and so support
> their removal. Probably should change developers-guide.
Ack. I mis-read. I thought he removed the IAE's themselves and not just
the declaration :) +1 on developers-guide mention.
Hen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Stephen Colebourne <sc...@btopenworld.com>.
----- Original Message -----
From: "Henri Yandell" <ba...@generationjava.com>
> On Tue, 7 Jan 2003, Sean Schofield wrote:
> > 1) I removed two of the overloaded getInstance() methods that took
> > DateFormatSymbols as arguments. SimpleDateFormat does allow a
> > DateFormat to be constructed using DateFormatSymbols but I thought for
> > now it might be nice to leave this out. It could be argued, however,
> > that we should include support for this, since some have described this
> > class as a thread-safe replacement for SimpleDateformat. (Actually the
> > changes related to this were commented-out for now so we can undo them
> > if people want.)
>
> If there's no reason as to why they're a pain to keep in, would be nice to
> not get some javadoc-lawyer hitting us with the reason why FastDateFormat
> isn't a SimpleDateFormat replacement.
Its not a SimpleDateFormat drop-in as it doesn't extend Format. It should do
(not the parsing, just the formatting part - throw
UnsupportedOperationException from the parsing)
> > 2) Removed 'throws IllegalArgumentException' from several methods. IMO,
> > runtime exceptions should not be declared in the method. I checked
> > several other classes in commons-lang and it seems that this follows the
> > current commons-lang coding convention as well.
>
> I think this is quite debatable and up-in-the-air. You're not shattering
> any rules by not having them in there, but there's a definite school of
> thought in favour of them being there.
I oppose putting runtime exceptions in the throws clause, and so support
their removal. Probably should change developers-guide.
> > 3) Changed the "style" argument in getDateInstance() and
> > getTimeInstance() from Object to int since the method was converting
> > everything to int anyways. (Forgot to do this for getDateTimeInstance()
> > but this will suffice for now as I'm not even sure we want to keep these
> > methods in this class [see next section].)
>
> Again, depends on how much we care about being a drop-in replacement.
>
> > Proposed/Anticipated Changes
> > ============================
> >
> > 2) Eliminate getDateInstance(), getDateTimeInstance(), and
> > getTimeInstance() from FastDateFormat. I think the goal behind these
> > methods is fine but I don't think they belong in this class which is
> > supposed to be a light-weight and thread-safe SimpleDateFormat
> > replacement. Earlier we had discussed the ability to have some common
> > date/time formats available in DateUtils. This is where I propose this
> > functionality be moved to.
>
> Assuming we don't advertise as a drop-in replacement, I like the DateUtils
> idea. As usual, I'm easily swayed by the opinion of the coder closest :)
> Dunno about others.
I don't see any great advantage to separating the factory from the class
(they don't separate them in JDK).
I propose the style would be FULL, LONG, MEDIUM, SHORT, ISO, ISO_FULL. They
should be defined using the Enum subpackage. (ISO_FULL includes
milliseconds)
Also, there should only be one constructor, and it should be protected in
case someone else wants to subclass.
> > 3) If we don't eliminate the methods in proposal #2, then we need to
> > investigate them for thread-saftey. They are all using SimpleDateFormat
> > methods and its been mentioned that some of these methods are not thread
> > safe (not sure if there is a problem with the specific ones used here.)
>
> Ah. Good enough reason to eliminate them. Just got to avoid the drop-in
> catchphrase.
I've taken a look and the calls should be threadsafe. They just get the
pattern. Its the actual formatting thats not threadsafe.
Stephen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [lang] FastDateFormat Update
Posted by Henri Yandell <ba...@generationjava.com>.
On Tue, 7 Jan 2003, Sean Schofield wrote:
> Changes already made:
> =====================
>
> 1) I removed two of the overloaded getInstance() methods that took
> DateFormatSymbols as arguments. SimpleDateFormat does allow a
> DateFormat to be constructed using DateFormatSymbols but I thought for
> now it might be nice to leave this out. It could be argued, however,
> that we should include support for this, since some have described this
> class as a thread-safe replacement for SimpleDateformat. (Actually the
> changes related to this were commented-out for now so we can undo them
> if people want.)
If there's no reason as to why they're a pain to keep in, would be nice to
not get some javadoc-lawyer hitting us with the reason why FastDateFormat
isn't a SimpleDateFormat replacement.
> 2) Removed 'throws IllegalArgumentException' from several methods. IMO,
> runtime exceptions should not be declared in the method. I checked
> several other classes in commons-lang and it seems that this follows the
> current commons-lang coding convention as well.
I think this is quite debatable and up-in-the-air. You're not shattering
any rules by not having them in there, but there's a definite school of
thought in favour of them being there.
Lang needs to standardise its approach here.
> 3) Changed the "style" argument in getDateInstance() and
> getTimeInstance() from Object to int since the method was converting
> everything to int anyways. (Forgot to do this for getDateTimeInstance()
> but this will suffice for now as I'm not even sure we want to keep these
> methods in this class [see next section].)
Again, depends on how much we care about being a drop-in replacement.
> 4) Changed Brian's Pair.java to be a private static inner class of
> FastDateFormat.
+1. Good move.
> Proposed/Anticipated Changes
> ============================
>
> 1) Clean up source code (including javadoc) to conform to commons-lang
> style. So fart this has been done with my changes only in an attempt to
> get things checked in and get some feedback. The code should be cleaned
> up as we polish this (and the test class.)
Fart's a good word here. Don't worry if you have to ask about a
style-point and it launches an argument. As long as we add decisions to
the developer's guide, each point is worth it.
> 2) Eliminate getDateInstance(), getDateTimeInstance(), and
> getTimeInstance() from FastDateFormat. I think the goal behind these
> methods is fine but I don't think they belong in this class which is
> supposed to be a light-weight and thread-safe SimpleDateFormat
> replacement. Earlier we had discussed the ability to have some common
> date/time formats available in DateUtils. This is where I propose this
> functionality be moved to.
Assuming we don't advertise as a drop-in replacement, I like the DateUtils
idea. As usual, I'm easily swayed by the opinion of the coder closest :)
Dunno about others.
> 3) If we don't eliminate the methods in proposal #2, then we need to
> investigate them for thread-saftey. They are all using SimpleDateFormat
> methods and its been mentioned that some of these methods are not thread
> safe (not sure if there is a problem with the specific ones used here.)
Ah. Good enough reason to eliminate them. Just got to avoid the drop-in
catchphrase.
> 4) Consider replacing use of HashMap due to the problem that Dick has
> mentioned.
Damn. Now you make me have to pay attention to 2 emails at once. No clue
which one was this though.
If it was synchronisation, Collections.synchronizedMap(new HashMap()) ?
Hen
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>