search

dolphinsmalltalk / Dolphin

Dolphin Smalltalk Core Image
MIT License
301 stars 58 forks source link

Converting Date to DateAndTime then to asUnixTime results in value that when converted back does not equal original #1193

Closed vince-refiti closed 8 months ago

vince-refiti commented 10 months ago

Describe the bug

today := Date today.
ut := today asDateAndTime asUnixTime.
self assert: [ today = (DateAndTime fromUnixTime: ut) asDate ]

To Reproduce Reproduced in Dolphin release/7.1

Expected behavior Assert should not fail. Exact code works in Pharo 11

Screenshots If applicable, add screenshots to help explain your problem.

Please complete the following information): Windows 11

Additional context Add any other context about the problem here.

blairmcg commented 10 months ago

This is because legacy Dates in Dolphin follow the Smalltalk-80 design so are not timezone aware and some historic choices were made for the conversions that are not consistent.

Let's work through what happens here, assuming that our local timezone is UTC+8, and we are executing this at 16:00 UTC.

1) Date today gives you today's date in local time. This is 31/10/2023 locally, but actually 30/10/2023 in UTC. 2) You now convert a Date without specifying a time, to a DateAndTime. At this point a choice has to be made as to the timepoint in the day the date is supposed to represent. Historically it was generally assumed (although unspecified, so this depended on the choice of the programmer) that Date was a date in local time. Therefore Date>>#asDateAndTime converts to a DateAndTime that is midnight (i.e. the start of the day) in the local timezone. Therefore the DateAndTime result is 2023-10-31T00:00:00+08:00, which is equivalent to 2023-10-30T16:00:00+00:00. 3) You convert the DateAndTime to a UTC representation (as a Unix time, but that doesn't really matter). Effectively the new DateAndTime is Date today asDateAndTime asUTC i.e. 2023-10-30T16:00:00+00:00 4) You now ask the UTC DateAndTime to convert to a Date. Now there is another choice to be made. Should the conversion convert back to a local Date, i.e. (DateAndTime fromString: '2023-10-30T16:00:00+00:00') asDate => 30 October 2023? Or would that be surprising to those who expected the same date as displayed by the DateAndTime? As DateAndTime>>#asDate comment says, the implementors interpretation of "least surprise" was the latter:

    "Answer a <Date> representing the date of the receiver. The <Date> will be that at the receiver's offset, so:
        - if a date in the local timezone is needed, send #asLocal first (i.e. `dateAndTime asLocal asDate`)
        - if a UTC date is needed, send #asUTC first (i.e. `dateAndTime asUTC asDate`)."

As the TZ of a Date is unspecified, this isn't necessarily wrong, even if it breaks the round trip. Its a choice that was made to satisfy a different expectation.

FWIW I think it was the wrong choice. It would be better to stick to the convention that Date and Time always represent local time. However the legacy Date and Time classes are only present for backwards compatibility, and for that reason alone it would be wrong to fix this because it would break compatibility with the legacy behaviour.

In Dolphin you should use DateAndTime exclusively. You then have an unambiguous point in time.

Pharo's Date is a different (and better) thing than the old Smalltalk-80 Date. It actually holds a DateAndTime, so it is TZ aware, and it also has the notion of a duration, i.e. its not just a point in time, but a span. I can see that might be useful, but the Date and Time classes in Dolphin have no use other than for backwards compatibility.

blairmcg commented 10 months ago

I changed my mind. We should at least fix this in 8. Still not sure about 7.1 - that needs some research of the history.

vince-refiti commented 10 months ago

Hey Blair

Thanks for the explanation.

Vince