datetime review part 2 [Update 4]

[Jonathan M Davis]

On Wednesday, November 10, 2010 15:03:11 Tomek Sowiński wrote:
> Jonathan M Davis napisał:
> > Latest: http://is.gd/gSwDv
> 
> My 2 cents:
> 
> Units of time are represented more naturally by an integer enum (could be
> anonymous) than a string.
> 
> A problem recurring in many methods:
> 
>     /+ref+/ Date opOpAssign(string op, D)(in D duration) nothrow
>         if((op == "+" || op == "-") &&
>            (is(Unqual!D == Duration) ||
>             is(Unqual!D == TickDuration)))
>     {
>         static if(op == "+")
>         {
>             static if(is(Unqual!D == Duration))
>                 return addDays(convert!("hnsecs",
> "days")(duration.total!"hnsecs")); else static if(is(Unqual!D ==
> TickDuration))
>                 return addDays(convert!("hnsecs",
> "days")(duration.hnsecs)); }
>         else
>         {
>             static if(is(Unqual!D == Duration))
>                 return addDays(convert!("hnsecs",
> "days")(-duration.total!"hnsecs")); else static if(is(Unqual!D ==
> TickDuration))
>                 return addDays(convert!("hnsecs",
> "days")(-duration.hnsecs)); }
>     }
> 
> If you're static if'ing each statement, it's a good sign the method should
> be broken up into two overloads. BTW, what was the problem with returning
> by ref?

I don't see any real benefit in splitting a function into two overloads rather 
than using static ifs in this manner. The resulting source code is likely nearly 
identical. You only end up with one function to document this way, and in this 
case the user doesn't necessarily care about what type of duration they're 
dealing with. If, they want to add or subtract a duration, here's the function 
to do it. If anything, I think that I tend to favor using static ifs over 
duplicate functions with different template constraints if there's no real 
difference in the from the users perspective.

As for the ref, there are several bugs relating to that. One I remember right off 
the top of my head is that you can't have a template function return by ref for 
some reason. I think that there's at least one related to invariants too, though 
the one about invariants not being able to be pure has been fixed which has 
reduced some of the invariant-related problems. A number of bugs relating to 
const-correctness, purity, and invariants have caused me a lot of headaches when 
working on datetime.

> Finally, bordering on bikeshed, weekdays' names are long and months' are
> short. Having both short and long names for weekdays and months would
> solve the inconsistency and satisfy everyone.

I hadn't thought about that. I'll think about it. I'm not sure that it really 
matters much though. I tend to prefer full names over short names, but that can 
become tedious when you use them a lot.

> Are there any plans for a Calendar interface to allow user implementations
> that distinguish business days and holidays?

I'm afraid that I have no clue what you're talking about. I mean, I know what a 
business day is and what a holiday is, but I'm not sure what you mean by a 
calendar interface in this context. Are you looking for a way to query whether a 
particular day is a business day, weekend day, or holiday? That strikes me as 
being a function rather than an interface, and it would be locale-dependent 
enough that I can't see it making into the standard library.

> > Also, since invariants can now be marked pure, I marked some of the
> > invariants pure and increased the number of functions marked as pure.
> > Unfortunately, due to the fact that many Phobos functions still aren't
> > pure (to!() and format() in particular), as well as bug #5191, many
> > functions that should be pure, aren't. But we're closer. Also, bug #4867
> > continues to prevent SysTime from being able to be immutable, and a
> > couple of bugs regarding invariants and templates make it so that a
> > number of functions that should return ref this, can't.
> 
> Maybe I'm late on the 'pure' changes, but how come all the setters are
> pure? I mean it modifies the (hidden) argument of the function, so it
> shouldn't be, no?

They'd be weakly pure rather than strongly pure. Ultimately, I believe that the 
only functions in std.datetime which should be non-pure are the ones which 
access global variables or make a call whose potentially result changes every 
call (like getting the time) or functions who indirectly call such functions. 
Some of those will be strongly pure and some won't, but it's a royal pain get 
much in the way of strongly pure functions without weakly pure ones. That 
suggestion of Don's was fantastic and will quite possibly make pure useable.

Weakly pure functions are functions which don't access globals or return a 
different result with each call but which can't be optimized out, whereas 
strongly pure functions are the same except that all of their parameters are 
immutable or can be implicitly converted to immutable, and they _can_ be 
optimized out in some cases. So, the getters in many cases would be strongly 
pure, whereas the setters would be weakly pure.

- Jonathan M Davis