[phobos] Split std.datetime in two?

[Jonathan M Davis]

On Friday 11 February 2011 00:50:08 Andrei Alexandrescu wrote:
> On Feb 11, 2011, at 12:24 AM, Jonathan M Davis <jmdavisProg at gmx.com>
> 
> wrote:
> > On Thursday, February 10, 2011 14:28:22 Andrei Alexandrescu wrote:
> >> On 2/10/11 11:33 AM, Jonathan M Davis wrote:
> >>> Well, as Steven says, most of std.datetime is unit tests. And
> >>> probably
> >>> over half of the rest is documentation, since most of the functions
> >>> themselves are relatively short and simple.
> >> 
> >> That means there are too many unit tests. There is such a thing as
> >> too
> >> much of a good thing.
> >> 
> >>> Add to that that I tend to like a fair bit of
> >>> vertical space in my  code (e.g. return statements pretty much
> >>> always
> >>> have a blank line above them), and how I code tends to take up a
> >>> fair
> >>> bit more LOC than you would without actually being more code.
> >> 
> >> As I mentioned, it would be courteous to your other colleagues if you
> >> were more economical with vertical space. Many people (and most of
> >> Phobos devs) consider vertical space a rare commodity.
> >> 
> >>> It wouldn't surprise me that if I
> >>> were to format std.algorithm in a manner that I'd typicall write
> >>> it, it
> >>> would at least double in line count simply due to additional blank
> >>> lines
> >>> that I'd introduce for code legibility.
> >> 
> >> I consider legible code compact code that I can keep on one screen
> >> and
> >> understand what it does.
> > 
> > There aren't very many functions in std.datetime which wouldn't fit
> > one one
> > screen. Regardless, it's obvious that code legibility is a
> > subjective thing.
> > 
> >>> And, of course, if I'd written std.algorithm, it
> >>> would almost certainly have had quite a few more unit tests. None
> >>> of that
> >>> increases the size of the actual binary.
> >> 
> >> It's not the size of the binary I'm worried about.
> >> 
> >>> I don't think that std.datetime is the
> >>> largest module in Phobos, but I do think that comparing its line
> >>> count to
> >>> something like std.algorithm really tells you how much of a
> >>> difference it
> >>> makes in the size of the binary - especially when so much of it is
> >>> unit
> >>> tests which wouldn't end up in a binary.
> >> 
> >> One thing is certain: someone who wants to take a look at
> >> std.datetime
> >> has the prospect of navigating 34KLOC.
> >> 
> >>> The unit testing functions I wrote are pretty much in the same
> >>> boat. They
> >>> had thorough unit tests - testing not only all of its code paths
> >>> (which
> >>> is what the % code coverage gives you) but an appropriate range of
> >>> values. The functions themselves weren't all that long.
> >> 
> >> They are unrolled loops that should be rolled. Most likely they
> >> test the
> >> same thing several times.
> > 
> > Some of them could be easily turned into loops - others not (most
> > probably not).
> 
> There was not ONE I looked at that couldn't be reduced by using a loop.

What I meant is that there are a lot of the tests which couldn't be turned into 
loops by incrementing the parameters. If you turn them into arrays of
parameters to send to the same function, then yes, you can loop them.

> > They don't generally quite test a set of consecutive values, though
> > there is
> > often a pattern to them.
> 
> They apply the same call to different values and check results. Loop.
> 
> > I tend to dislike doing much complicated just to try
> > and reduce the lines in a unit test, because the risk of screwing it
> > up goes up,
> > and if the tests are wrong, the code will be wrong. So, the tests
> > are simple but
> > repetitive.
> 
> I don't buy this argument at all. What, are simple loops difficult all
> of a sudden? Then, one should strive to make all code simple; the
> argument is not for unittests only. The unittest scope is not a
> license to throw all good practices out the window and write bloat.

One of my key problems with using a loop in a unit test is that if you screw up 
the loop's body, _everything_ that that unit test is testing is incorrectly 
tested. Granted, if all you do is copy-paste the loop's body, you can run into 
the same problem, but by rewriting similar tests multiple times, such mistakes 
are more likely to be caught.

> > The risk of screwing up like that for std.datetime is lower at this
> > point, since
> > it has already been thoroughly tested (the likelihood of a test
> > failure when
> > changing the tests is now much more likely to be a screwed up test
> > rather than
> > the code being broken). So, I'd be far more willing to try to alter
> > the tests to
> > take up less space now that I would have been when writing it.
> 
> Not buying this any more than the other. Good unittests are as goo as
> bulky unittests - in fact way better.
> 
> > I suppose that I could look at trying to turn some of the tests into
> > arrays
> > which get looped over. That sort of thing is not always going to be
> > possible
> > though, regardless of how repetitive the tests may seem.
> 
> I'd be hard pressed to find unittests tha are not, in fact, loops.
> 
> > And while the functions
> > aren't generally long, and _most_ of them are simple, there _are_
> > some of them
> > with calculations which are easy to screw up, and a lot of tests are
> > necessary
> > to catch edge cases and the like. For instance, at one point, I
> > thought that I'd
> > finally gotten the gregorian day calculations right only to find
> > that when I added
> > tests for negative years which ended in 99, the result was always 1
> > day off. I
> > only caught it because I had a lot of tests.
> 
> Loops would have caught those too.
> 
> > While I'm fine with the tests as they are, obviously others aren't.
> > So, I'll see
> > if I can reasonably reduce how many lines they take up. But I'm not
> > particularly
> > willing to reduce what's being tested.
> 
> Jonathan, you are definitely right in principle. It's just the
> frightening proportions you are taking this to. Unittests are great.
> 10x unittests are not. I feel we cannot count here on some simple
> commonsense limits that would normally need no debate.

Well, I don't agree with you. I prefer to _avoid_ loops in unit test code and 
make it insanely simple. I think that that is less error-prone and that it leads 
to better unit testing code. I also prefer to have a lot of unit tests. I think 
that it has led to solid code in std.datetime.

However, I do acknoweldge that most people here disagree with me and think that 
the length of the unit tests in std.datetime is excessive and problematic. So, I 
will take time sometime relatively soon and work on making them take less space. 
And any future anything that I propose to Phobos, I will work on minimizing the 
amount of space that its unit tests take up before I propose it.

- Jonathan M Davis