Hi Dave,

I finally had some time to dig into the code and the tests.

On Sun, 13 Mar 2016, David T. Lewis wrote:

> On Sat, Mar 12, 2016 at 08:48:21PM +0100, Levente Uzonyi wrote:
>> Hi Dave,
>>
>> I ran the tests and found that there are three types of failures.
>> 1. Some tests assume that the default offset is #localOffset. I don't
>> think it's correct, but that's how is it in Trunk.
>
> There are 10 failing tests in my image. These are cases in which either
> I was uncertain how to handle the issue, or where I thought that the
> current behaviour in trunk may be wrong.
>
>> 2. Hashes have changed. This implies that HashedCollections containing
>> DateAndTime instances as key will have to be rehashed. In the base Trunk
>> image there are appear to be no such HashedCollections, but not rehashing
>> such collections will break images with custom packages in them after
>> update.
>
> I am not expert in this area at all. But part of the update process
> (in the MC postscripts) finds all instances of the old style DateAndTime,
> and becomes them into the new ones. Would the HashedCollections contianing
> DateAndTime instances as key still require rehash in that case?

Yes, it is. When their hash changes, these objects will not be found in 
such collections.

>
> Related to DateAndTime>>hash, I would also like to ask for guidance as
> to how DateAndTime>>= should be defined. Maybe this is an ANSI Smalltalk
> question, but I was not sure if we should be thinking of DateAndTime as
> a magnitude, such that two instances are equal if their utcMicroseconds
> are the same? Or are two instances equal if they have the same magnitude
> and also the same localOffsetSeconds?
>
> I implemented the first approach (two instances are #= if they represent
> the same UTC time, regardless of timezone). There may be use cases
> where the other approach makes sense. But my thinking was that when
> comparing two instances of DateAndTime, they should not be considered
> the same unless they both represent the same point in time, hence UTC
> time regardless of the time zone in which the instance was created.

As we discussed this at the last board meeting, the right thing is to use 
UTC time only both for #hash and #=, so this is correct.

>
>> 3. Loss of resolution and strict monotonicity. In Trunk [DateAndTime now <
>> DateAndTime now] is always true. This is possible because DateAndTime has
>> nanosecond resolution, but the source clock only has microsecond
>> resolution, and our machines are so slow that it takes more than a
>> nanosecond to execute the primitive. All these allow us to use the
>> sub-microsecond part of the timestamp to create intermediate strictly
>> monotonic values.
>>
>
> To be honest, I do not fully recall the rationale for this. One
> important case is that of the system clock being set back due to
> ntp synchronization. Fiddling with the nanoseconds number to make
> it appear as if the OS had reported an increase in time (when in
> fact it did not) seems questionable to me, but I know that it was
> done for some good reason. I do remember the discussions, so I
> should look back through the squeak-dev archives and find out the
> reason this was done.

It comes handy in many situations where uniqueness is a must. E.g. 
creating file names, storing them in a database. I don't know why it was 
added originally.

The loss of the nanosecond part has another side effect. utcMicroseconds 
will be a Fraction when the resolution of the parsed input is too high. 
This is somewhat compatible, but it makes things slower. E.g.:

'2002-05-16T17:20:45.000000009+01:01' asDateAndTime utcMicroseconds
"==> (1021565985000000009/1000)"

I found two other issues:

DateAndTime class >> #date:time: ignores the nanoseconds from the time 
argument. This is present in the Trunk as well, but no test fails, because 
in Trunk TimeStamps have their nanoseconds part cleared (in a very 
inefficient way).

The other issue is that DateAndTime >> #getSeconds returns a fraction, 
which contains the nanosecond part as well. This makes DateAndTime >> 
#asTime return a Time instance which has its nanoseconds part doubled.
I simply changed #/ to #// in #getSeconds to work that around. This 
also satisfied the expectations of all other senders of #getSeconds, but 
then it turned out to be the same as #getWholeSeconds.
Since #getSeconds is a replacement for the original seconds variable (if 
I'm not mistaken), I think it's safe to do this change and replace sends 
of #getWholeSeconds to #getSeconds.

I found that I'm a developer on the project, so I have uploaded a new 
version with the changes described above + some optimizations.

About some #FIXMEs:
#offset: is plain wrong according to its comment. It should simply create 
a copy with the new offset and the same utcMicrosecond value. Just like 
how #utcOffset: does. We should keep one or the other but not both.

Levente

>
>> Levente
>>
>
> Thanks very much for looking at this. I added you and Chris Muller as
> developers on the UTCDateAndTime repository on squeaksource.com in case
> you want to make any changes.
>
> Much appreciated,
>
> Dave
>
>
>