On Fri, Apr 22, 2022 at 12:10:16PM -0700, Tim Johnson wrote:
> 
> > On Dec 9, 2021, at 9:06 AM, Ron Teitelbaum <ron at usmedrec.com> wrote:
> > Date is a Timespan while DateAndTime is not. I think we should drop the timezone offset when converting a DateAndTime to a Date. Yet, the class comment of Date reads, that there are rare cases where one would need such an offset. Not sure.
> > 
> > In my opinion, The result of the comparison should only consider the result of the date with the offset applied.  While it is true that the original date + offset might result in a different date the result should still be true if the resulting dates are the same.  One is the result of the date plus the offset the other is a date but they are both Functionally equal.  The results currently are converting both dates to UTC before the comparison.   
> > 
> >        self asUTC hasEqualTicks: aDateAndTimeOrTimeStamp asUTC
> > 
> > That to me is the error.  If you wanted to make that comparison you would do asUTC yourself to remove timezone information.  
> > 
> > I can not fathom a reason that DateAndTime today asDate = Date today.  is better by returning false.  Anyone worried about timezones in this instance would use UTC or local conversions first before doing the comparison. 
> 
> I just found an old 5.1 image where I was playing around with something similar.  
> 
> | x c y |
> x := TimeStamp now.
> c := String streamContents: [:s | x storeOn: s].
> y := TimeStamp readFrom: c readStream.
> x = y   " => false  "
> 
> whereas:
> 
> | x c y |
> x := TimeStamp now asUTC.
> c := String streamContents: [:s | x storeOn: s].
> y := TimeStamp readFrom: c readStream.
> x = y   " =>  true   "
> 
> The difference being that "Timestamp now" has localOffsetSeconds of -25200 (where I live), but this offset is not getting stored via TimeStamp>>#storeOn: (which produces '''22 April 2022 11:48:17.542009 am'' asTimeStamp' ).
> 
> I suspect this could be avoided if Squeak were using ISO 8601 formatting for TimeStamp>>#storeOn:   which might capture local time zone info using + or T or something at the end of the string.  But then the code for TimeStamp class>>#readFrom: eventually hits Time class>>#readFrom: which does not support offsets / time zones either.
> 

TimeStamp>>#storeOn: is arguably broken in the sense that it does not work
as described in Object>>#storeOn: although it does pass its own unit test
in TimeStampTest>>#testStoreOn.

A simple solution might be to just remove the TimeStamp>>#storeOn: method
and update the unit test, since the superclass implementation looks fine.
But I don't know if there might be side effects. For example, time stamps
are likely used in various database applications that might rely on the
current #storeOn: method.

> Reading what Ron wrote above, it seems that the onus is on the user of these classes to remember to use #asUTC, which seems common/best practice...  and avoids unexpected(?) results like the above.
> 

Yes, I think that's right.

Regarding the "DateAndTime today asDate = Date today ==> false" concern,
I think Ron is right that our current implementation does not make sense.
But I would not know how to fix it without a clearer understanding of
what a "Date" is in Squeak. Its duration either begins at midnight in
some time zone or it doesn't, it can't be both things.

By the way, if anyone is interested in comparing our current Date and TimeStamp
implementation to the earlier versions circa Squeak 3.6, try this:

	(Installer ss project: 'ST80Date') install: 'ST80Date-dtl.12'

Then inspect these:

	ST80TimeStamp current.
	ST80Date today.
	ST80Time now.

I added the old TimeStamp implementation (renamed as ST80TimeStamp) just
yesterday so you can see the differences. The package can be safely added
and removed without any effect on your real Chronology classes.

Dave