DateRange immutable?

[rapand]

Hi

Frankly speaking I am new to JFreeChart, but to me it 'seems' as a bug when DateRange claims to be immutable when the public constructor DateRange (Date lower, Date upper) is using these Date objects when calling either getLowerDate or getUpperDate since Date is not immutable.

Two ways to solve this would be to:
1. call clone on the lowerDate and upperDate when the get methods are called.
or
2. using: e.g. return new Date (use super/Range's getUpperBound () method) for getUpperDate likewise for getLowerDate - since then the responsibility lies only at one place (in Range).
Well I do think it is required that each call to these get methods spawn a new Date object to get this truly immutable.

- Perhaps just for the eye make fields lower and upper in Range final.

Thank you for what seems to be a great library!

[rapand]

Hi again

FixedMillisecond crossed my way as well. It seems that the constructor public FixedMillisecond(Date time) and public Date getTime() also have problems fulfilling that the FixedMillisecond class is immutable as stated by the comment of the class.

Thank you for reading!

[jpmaia]

The Java Date class IS immutable. Where did you get the idea it isn't?

Edited: after the subsequent post, am I missing something?

Edited still later: I went back and double-checked the Java doc on Date. The Date class is not immutable as you've stated. Now, where did *I* get the idea that it was immutable? *sigh*

[david.gilbert]

Yes, you are right about this - thanks for reporting it. I'll fix it.

[rapand]

Thank you David!

I'm sorry to disturb you once again - when I had some more time yesterday I went through org.jfree.data.time.

1. I noticed that SimpleTimePeriod has the same problem.

2. This might only be a less important issue (I guess), but the Day class contains fields that are defined as e.g.: protected static final DateFormat DATE_FORMAT_SHORT

Since DateFormat has at least 4 'set' methods even sub-classes may change global behaviour of the different instances.
(So this is not related to immutability since compareTo and hashCode does not seem to depend on them but more because parseDay of Day depends on it and are related to fields DATE_FORMAT and DATE_FORMAT_SHORT (the other fields for medium and long formats does not seem to be used but might be in subclasses).)

I guess not making the fields static would solve this behaviour depending on the importance of this in relation to parseDay.

3. An also not so important issue is that in TimeSeries removeAgedItems (long latest, boolean notify) TimeZone.getDefault () is used.
Now since TimeZone.setDefault (...) sets the TimeZone globally - one 'could' potentially make use of Dates written in one TimeZone an in accident delete those in another TimeZone where they shouldn't, but how often does it happen that such a case pops up? I do not know - unless you perhaps serialize the objects to another location on the Globe Perhaps graphs used in banks simultaneously at different locations - well only speculations.

I admit I'm not into this TimeSeries class, but I noticed that you in some other classes save the default TimeZone during construction e.g. in RegularTimePeriod: public static final TimeZone DEFAULT_TIME_ZONE = TimeZone.getDefault(); - perhaps that is a simple and good fix?

[jpmaia]

As far as saving an instance of TimeZone.getDefault....

This shouldn't create a problem in most applications, but if you are in an application where the user can choose their time zone on the fly (as is the case with the application I work on), it could be a bad thing to have various time zone instances saved off in static or instance variables in the code.

Dates stored as long values are all calculated based on GMT, so the local time zone really doesn't apply. But when you work with the Calendar class and the translation of long values into a presentable date, this is where a saved static time zone reference could be at odds with a subsequent time zone choice by the user.

[seh4nc]

The new DateRange resulting from this change (r1081) causes my application to spend a large amount of time and memory creating new Date objects from inside the DateAxis.valueToJava2D() method.

Can we rethink this change? I understand java.util.Date is not immutable but a lot of interfaces return (non-cloned) collections, arrays, and Dates for performance reasons, you just have to trust the caller to respect the immutability contract.

Edited: I'm referring to change to DateRange made in response to the original DateRange immutability issue, not to the TimeZone issues raised later.

[david.gilbert]

I'm wondering if this problem could be fixed by adding getLowerMillis() and getUpperMillis() to the DateRange class, and then the valueToJava2D() method can work directly with the millis instead of creating Date instances?

[david.gilbert]

Seems to work, so I've committed the change to Subversion for inclusion in the 1.0.11 release.

[seh4nc]

Sounds like an excellent solution. Thanks Dave!