ocaml-community / calendar

OCaml library for handling dates and times.
Other
42 stars 9 forks source link

API changes: avoid relying on global state? #8

Open c-cube opened 6 years ago

c-cube commented 6 years ago

there are some global mutations in the lib (e.g. Time_zone.change) which make reasoning about the library complicated.

I think it'd be nice to instead have a ?tz:Time_zone.t argument in functions that make use of the global state, to overload it.

pmetzger commented 6 years ago

This seems reasonable, though could we get rid of global state entirely?

c-cube commented 6 years ago

I think there's room for discussion there:

The optional parameter is a smoother transition path, but it's true that we need to know where we want to go eventually.

pmetzger commented 6 years ago

I'm not sure it's the best idea, but can one create a functor parameterized over a datum like a timezone (rather than another module) so you get a set of functions specialized to a particular timezone? (You can certainly do a poor man's version of this with lexical closures.) Forgive me for asking what might be a stupid question, although I've been using OCaml for a year or so every once in a while a question like this comes up in my head from lack of knowledge.

loxs commented 6 years ago

I personally like the idea of a functor. It means that not all code will have to be rewritten but only when opening the module, which will now be equivalent to setting the time zone in the current implementation. To me this is a reasonable change for a major version bump.

Drup commented 6 years ago

My vote goes to trying to remove the global state has much as possible, even if it makes breaking the API. It's really a distasteful mechanics that generates bugs far too easily. In general, dates should all be in UTC anyway. only the printing should be in another timezone.

pmetzger commented 6 years ago

It is usually best if things in date libraries are maintained internally in UTC or TAI, yes.

I'm not sure that a functor of the type I'm mentioning above is even possible, I've never seen it done. The equivalent is easily done with lexical closures, but this isn't something I've observed in other OCaml code. It was more of a question about whether it's a thing that can be done or not.

loxs commented 6 years ago

Yes, probably what @Drup suggests is the best way forward.

c-cube commented 6 years ago

Could the dates embed the timezone in which they were defined? I'm really no expert but it conveys the intent better and may be more robust in some cases.

For example if you live in Europe, set an alarm every morning at 7, and go to a conference somewhere in the US, storing the alarm event in UTC would be wrong?

Of course, comparing dates, or other operations, could convert them to UTC first. I think this kind of design decision would be helped by looking at what other languages do.

loxs commented 6 years ago

This example with the alarm is another type of time data type, which is “without timezone” and just depends on the context. It’s not in UTC, it’s in no time zone at all. And yes, I think we need to support this (if not already supported)

On the other hand, when the time is “zone aware”, storing it in UTC always is what I have been doing and have read on the topic, and generally never had issues with this way. Of course, I am also not an expert (yet) so yes, I’ll have a look at various languages and databases (where I have mostly dealt with date and time)