--- On Mon, 2/15/10, Adrian Crum <[email protected]> wrote: > From: Adrian Crum <[email protected]> > Subject: Re: TimeDuration is not immutable > To: [email protected] > Date: Monday, February 15, 2010, 11:11 PM > --- On Mon, 2/15/10, Adam Heath > <[email protected]> > wrote: > > From: Adam Heath <[email protected]> > > Subject: Re: TimeDuration is not immutable > > To: [email protected] > > Date: Monday, February 15, 2010, 10:55 PM > > Adrian Crum wrote: > > > Adam Heath wrote: > > >> Adrian Crum wrote: > > >>> Adam Heath wrote: > > >>>> Adrian Crum wrote: > > >>>>> Adam Heath wrote: > > >>>>>> The javadoc for > > org.ofbiz.base.util.TimeDuration says it is > > >>>>>> immutable; > > >>>>>> however, it's fields are not > > final, and lots of methods modify it's > > >>>>>> fields. > > >>>>> The methods that modify fields > are > > directly or indirectly called by > > >>>>> the > > >>>>> constructor and are protected. > Let me > > know if any public method > > >>>>> modifies > > >>>>> a field. > > >>>> fromLong modifies the fields > directly, not > > from the constructor. > > >>> fromLong is a static method that returns > an > > immutable instance. > > >> > > >> Right you are, but the statement still > > stands. fromLong should use > > >> the constructor to set that. > > >> > > >>>> makeNegative modifies the fields. > > >>> makeNegative is protected and used > > internally. > > >> > > >> The the same statement for set() applies. > > >> > > >>>> set() is protected, TimeDuration is > not > > final, so sub-classes could > > >>>> change the fields. > > >>> If I need to extend TimeDuration, then I > might > > also need to change the > > >>> fields. > > >> > > >> If fromLong just used the constructor, after > doing > > math on the long, > > >> then it wouldn't need to call makeNegative > > itself. Then, the only > > >> places that would call set() or makeNegative > would > > be the constructor, > > >> so the code could be inlined. Once it is > > inlined, then the fields can > > >> be made final, and the class becomes a > proper > > immutable. > > >> > > >> Additionally, becase fromLong creates an > empty > > TimeDuration, then sets > > >> the fields one by one, the vm is free to > re-order > > those values, and > > >> even delay then for a *long* time, well > after > > fromLong returns to it's > > >> caller. There is no synchronization on > this; > > if these fields were > > >> only set from the constructor, then this > problem > > wouldn't occur. > > >> > > >>>> Ideally, this class should extend > Number, > > or some other number class. > > >> > > >>> Why? > > >> So we can do TimeDuration.longValue(), and > get the > > internal duration. > > > > > > What would it mean if you called > > TimeDuration.intValue()? What would > > > that value represent? > > > > > > In designing the API, I was trying to avoid > equating a > > duration with a > > > long value. The current support for long values > was > > added to accommodate > > > existing entity fields that store durations as > > longs. The long value > > > should not be confused with elapsed milliseconds > - it > > is a TimeDuration > > > *encoded* as a long. (Another change I never got > > around to was renaming > > > the parameters so there is no reference to > millis.) > > > > > > The whole idea of TimeDuration is that it is a > data > > type. It would be > > > nice to be able to do math on TimeDurations (X > > duration plus Y duration > > > equals Z duration), but at the same time, you > > shouldn't be able to do > > > math on the internal representation of the > duration > > (since the > > > implementation is supposed to be hidden). > > > > > >> Also, shouldn't the duration store the value > > internally as a long? > > >> That would solve the normalization problem, > and > > also reduce the memory > > >> usage. > > > > > > I don't recall my reasoning for that. I guess it > was > > for performance > > > reasons. If the internal value was a long, then > each > > accessor would have > > > to decode the long. > > > > > >> I'm willing to do all these changes; first > would > > be to have test cases > > >> and full coverage. > > > > > > That would be great! I created the class to make > > calendaring code > > > simpler. It could use some TLC. > > > > Ok, sorry to say this, but TimeDuration needs to be > > removed. > > com.ibm.icu.text.DurationFormat, which handles things > like > > 2 days from > > now and 3 hours ago, uses a long for the duration. > > > > Additionally, trying to duplicate leap year > calculation, > > there is not > > a whole number of days in a year(it's not quite > > 365.24). If long is > > good enough for icu, then why should we try to use > > something else? > > Grasshopper, > > TimeDuration is not concerned with such details. They are > delegated to the Calendar class. > > Tell me, using com.ibm.icu.text.DurationFormat - how do you > add two durations?
Better yet, I depart LAX 23:00 Feb 15 2010 (Gregorian calendar) and arrive in India (whatever arrival time and calendar is in Hindi). What was my flight duration? TimeDuration MUST be a data type. It doesn't work otherwise.
