[phobos] datetime review

[Jonathan M Davis]

On Sunday 10 October 2010 07:33:51 Andrei Alexandrescu wrote:
> On 10/9/10 19:57 CDT, Jonathan M Davis wrote:
> > I ended up making it fairly easy to _use_ in generic code, I believe, but
> > it doesn't do much to generate code, which does indeed result in a lot
> > of generic and repetitive code.
> 
> Libraries that contain type names in function names have trouble with
> generic code.

True, but while I do have functions such as addYears() and addSeconds(), I also 
have functions like addTUnit() which call those functions. So, the generic code 
has generic functions that it can call, but there are non-generic versions which 
are more pleasant for programmers to deal with in non-generic code (though just 
using durations solves the problem in this particular case).

> > For Clock, Dur, and IRange, however (which are similar namespaces), I
> > really do think that the added namespacing is well worth it. It makes
> > the code much clearer. Dur in particular would be bad to have its
> > functions separated out, because then you have functions like years(),
> > months(), and days() which are going to conflict all over the place with
> > variable  names.
> 
> I'm just wondering how come dates and times are the only API that's
> hairy enough to require carving namespaces within the whole Phobos. I'm
> just saying a solid argument should be brought up. Otherwise anyone
> could use this as a precedent to justify unnecessarily baroque APIs.

Well, for Clock, it makes it quite explicit that you're getting the time and it 
nicely organizes the various functions which deal with getting the time. You 
could choose not to have it, but it seems cleaner to me to have it.

For Dur, it avoids name conflicts since names like years, days, and hours are 
going to conflict with variable names and the like. And even if they don't 
conflict in a way that results in compilation errors, it's much easier to screw 
up and get logic errors if they aren't encapsulated in Dur. Now, if we change 
TUnit to use strings, then you can do something like Dur!"days"(5) and then you 
don't need the Dur class as a namespace anymore.

For IRange, it not only encapsulates the various range-generating functions, but 
it makes it quite clear in user code that that's what you're doing when you have 
IRange.funcName() - certainly clearer than just funcName().

I am by no means arguing that you should have such namespaces all over the place 
in Phobos, but I do think that they can be useful in some cases. I think that 
one of the major reasons that this sort of thing hasn't been done previously is 
because most modules either contain completely unrelated functions (such as 
std.algorithm) or _all_ of the functions are related (such as std.path). In such 
cases, namespacing some of them really doesn't make much sense.

> >> * As another random example:
> >> 
> >> DayOfWeek getDayOfWeek(DayOfGregorianCal day) pure nothrow
> >> 
> >> could/should be (in client use):
> >> 
> >> get!(TUnit.dayOfWeek, TUnit.dayOfGregorianCal)(day);
> > 
> > Since those functions would all have to be have specialized version of
> > such templates, I'm not sure how much you gain with that approach -
> > particularly when there aren't very many of those functions (and IIIRC
> > they're all package or private rather than in the API). But it may be
> > worth doing something like that. I'm more skeptical of this suggestion
> > than the others though.
> 
> What you gain is that the client only needs to remember "get" and the
> types, instead of the cross product of types. Not to mention advantages
> in generic code, although I don't have an example handy.
> 
> Fewer functions for a given functionality == better library.

I agree that in the general case, this sort of thing is useful, but in this 
case, we're only talking about code which is private/package to datetime and not 
used outside of it, and I believ that getDayOfWeek is the only get function like 
this which exists. That's why I don't see much point in templatizing it like 
that. If there were several of them and/or they were public, then that would 
make good sense. But it's a not a public function, and there's only one.

- Jonathan M Davis