Smalltalk › Squeak › Squeak - Dev

Status of date problem

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

26 messages Options

Max Leske

Re: Status of date problem

On 19 Mar 2016, at 14:25, Bert Freudenberg <[hidden email]> wrote:

On 19.03.2016, at 11:40, Max Leske <[hidden email]> wrote:

On 19 Mar 2016, at 08:49, [hidden email] wrote:

And Max’s tests might work again, too, without manual preference fiddling.

Yes, that fixes my problem. I still needed to make a small adaptation to reset the value of #automaticTimezone to the value used before the tests but that’s no issue.

Well … if we could, I’d like to automate this, too.

Does the test save the previous time zone value, and restore it after?

No, but it should, now that you mention it. Currently I save the #automaticTimezone value and restore it. Storing and restoring the time zone instead makes more sense since that will also work in Pharo (where, I realise now, this test actually modifies the current time zone without restoring it to the original value).

If that’s the case we might re-enable the automatic time zone on the restore operation.

Sounds reasonable. So #automaticTimezone would be set to true if #localTimezone == the actual local timezone.

- Bert -

Bert Freudenberg

Re: Status of date problem

On 19.03.2016, at 14:39, Max Leske <[hidden email]> wrote:

On 19 Mar 2016, at 14:25, Bert Freudenberg <[hidden email]> wrote:

On 19.03.2016, at 11:40, Max Leske <[hidden email]> wrote:

On 19 Mar 2016, at 08:49, [hidden email] wrote:

And Max’s tests might work again, too, without manual preference fiddling.

Yes, that fixes my problem. I still needed to make a small adaptation to reset the value of #automaticTimezone to the value used before the tests but that’s no issue.

Well … if we could, I’d like to automate this, too.

Does the test save the previous time zone value, and restore it after?

No, but it should, now that you mention it. Currently I save the #automaticTimezone value and restore it. Storing and restoring the time zone instead makes more sense since that will also work in Pharo (where, I realise now, this test actually modifies the current time zone without restoring it to the original value).

If that’s the case we might re-enable the automatic time zone on the restore operation.

Sounds reasonable. So #automaticTimezone would be set to true if #localTimezone == the actual local timezone.

Yes. I just committed Chronology-Core-bf.5 which works that way. If you save and restore the localTimeZone it will take care of enabling the automaticTimezone preference.

It does so by looking at the timezone’s abbreviation: if it is “LT” then the preference gets enabled, otherwise it gets disabled. So as long as your test does not use a timezone with that abbreviation, it should work.

- Bert -

smime.p7s (5K) Download Attachment

Chris Muller-3

Re: Status of date problem

In reply to this post by Bert Freudenberg

Woops, I see I made a typo, I said I wish to keep #nowWithOffset:, but
in fact, that's the one you (rightfully) deprecated. #nowAtOffset: is
the new one which I wish to keep.

Your concern seems to be either about the size of the API, or the
rightness of having #nowAtOffset: as a constructor.

My interest is for the API to be conventional and consistent, and by
that, I mean, the user should expect to be able to interact strictly
with the constructor methods to obtain the instance they want, as is
the case with all value objects, and not have to create an interim
instance first.

Clearly, the attributes of a DateAndTime are the ones settable by this
constructor, #year:month:day:hour:minute:second:nanoSecond:offset:.

offset: is the last argument. Its already established as a part of
constructor API. #nowAtOffset: simply mirrors that, except supplies
the "nowness" from the prim and lets the user vary the location. I
really feel this is a good API design to keep nowAtOffset:.

I actually think we should get rid of DateAndTime>>#offset:, or at
least make it a private (or initializing) method.

Finally, DateAndTime class>>#now:offset: should be made private. Its
private, right?

>>> Since “utcOffset:" appears to do the right thing I’d actually prefer it to the “nowAtOffset:” you just introduced. E.g. to get the current time in LA you could simply use
>>>
>>> DateAndTime now utcOffset: -7 hours
>>>
>>> No need for a new message. Arguably this is what “offset:” should do, and it would if the implementation was utc-based (as is David’s) rather than local-time based.
>>
>> I wish to keep #nowWithOffset:. DateAndTime's are "value" objects --
>> like Points and Fractions. I never want to write code that sets any
>> state on existing DateAndTime objects, not even their #offset.
>
> #offset: does return a new instance.
>
>> Value objects are only ever created, and then their state never change
>> in 99% of cases. Magma treats DateAndTimes as immutable in the
>> database for performance -- it doesn't bother to check if they
>> changed.
>
> Yep. Same in general.
>
>> Semantically, #utcOffset: doesn't mean anything to me. What is a "UTC
>> offset?"
>
> Agreed.
>
>> #nowWithOffset: sorts in the list next to #now, and makes
>> it abundantly clear to users what use-case it handles. I wish to keep
>> it.
>
> How about one character more? A space?
>
> ... now withOffset: ...
>
> I think composability is preferable to an explosion of one-off specialized methods. I'd rather deprecate both #offset: and #utcOffset: and start doing the Right Thing.
>
> - Bert -
>
>

Bert Freudenberg

Re: Status of date problem

On 19.03.2016, at 16:38, Chris Muller <[hidden email]> wrote:
>
> Woops, I see I made a typo, I said I wish to keep #nowWithOffset:, but
> in fact, that's the one you (rightfully) deprecated. #nowAtOffset: is
> the new one which I wish to keep.

Yes that’s the one I meant.

> Your concern seems to be either about the size of the API, or the
> rightness of having #nowAtOffset: as a constructor.
>
> My interest is for the API to be conventional and consistent, and by
> that, I mean, the user should expect to be able to interact strictly
> with the constructor methods to obtain the instance they want, as is
> the case with all value objects, and not have to create an interim
> instance first.

Okay, having that method as a possible optimization point is an argument in favor.

If you’re touching this, would you mind changing the implementation to this?

^self now utcOffset: offsetDuration

And we should think about deprecating utcOffset: and renaming it to ... atOffset:?

> I actually think we should get rid of DateAndTime>>#offset:, or at
> least make it a private (or initializing) method.

+1

(if we deprecate it, we need to adjust the message in nowWithOffset:)

> Finally, DateAndTime class>>#now:offset: should be made private. Its
> private, right?

Yes, making it private sounds right.

- Bert -

smime.p7s (5K) Download Attachment

Chris Muller-3

Re: Status of date problem

> If you’re touching this, would you mind changing the implementation to this?
>
> ^self now utcOffset: offsetDuration

Yes, I thought about that, until...

>
> And we should think about deprecating utcOffset: and renaming it to ... atOffset:?

... I thought that I did not want to add another sender of a message I
didn't much like (utcOffset:).

+1, I like that. #atOffset: can be thought of as a "converting"
method, same magnitude, just converted to be using a different "lense"
onto that magnitude (the offset).

>
>> I actually think we should get rid of DateAndTime>>#offset:, or at
>> least make it a private (or initializing) method.
>
> +1
>
> (if we deprecate it, we need to adjust the message in nowWithOffset:)

nowAtOffset:. :) Whew, I'm glad we're cleaningup the API a bit,
so this would be a good time to re-do the categories too. We don't
use categories to delineate "ansi" from "squeak" anywhere else, I
don't find it helpful at all. I'd like to make them more
conventional.

Bert Freudenberg

Re: Status of date problem

On 19.03.2016, at 17:43, Chris Muller <[hidden email]> wrote:

>>> I actually think we should get rid of DateAndTime>>#offset:, or at
>>> least make it a private (or initializing) method.
>>
>> +1
>>
>> (if we deprecate it, we need to adjust the message in nowWithOffset:)
>
> nowAtOffset:. :)

No, I meant the deprecation warning in nowWithOffset: which suggests to use "... now offset:".

- Bert -

smime.p7s (8K) Download Attachment