On Tuesday, November 09, 2010 15:48:30 bearophile wrote: > Yao G.: > > Ugh. The datetime.d file is 1.5 MB? 0_o
Heavy unit testing and documenting everything adds up. The implementation isn't all that small either, but it's the unit tests and ddoc comments which really increase the size. Most of that will never end up in any user program. > It contains too-much-repetitive code like: > > assertEqual(SysTime(DateTime(2010, 8, 1, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 733_985); > assertEqual(SysTime(DateTime(2010, 8, 31, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_015); > assertEqual(SysTime(DateTime(2010, 9, 1, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_016); > assertEqual(SysTime(DateTime(2010, 9, 30, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_045); > assertEqual(SysTime(DateTime(2010, 10, 1, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_046); > assertEqual(SysTime(DateTime(2010, 10, 31, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_076); > assertEqual(SysTime(DateTime(2010, 11, 1, 23, 59, 59), > FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_077); > > > My suggestions: > - Keep only 5-10% of the tests inside the datetime.d module, and move the > other 90-95% in a separated datetime test module. - Use deterministic > (repeatable) random data in a loop to perform some compact fuzzy testing - > Use alias or string mixing to greatly reduce the amount of text contained > in the tests. Tuples help a lot. All those lines may be replaced with a > single loop that contains something like: assertEqual(SysTime(DateTime(z0, > x1, x2, x3, x4, x5), FracSec.from!"msecs"(x6)).dayOfGregorianCal, x7) > Where the xn data comes from an array of typecons.Tuples like: > Data5(2010, 11, 1, 23, 59, 59, 999, 734_077) While there seem to be a lot of tests, they've turned out to be _really_ helpful and by their very nature, they _need_ to be repetitive. Testing a lot of stuff with slight differences is where errors are likely to be found. I've definitely found errors in my code where there were a lot of tests that were similar but one of them which was just a bit different from the ones next to it managed to find a bug due to whatever quirk in the logic made it a boundary of some kind in the calculation. Having all of those similar tests is valuable. Now, some of those could be turned into loops, but that would complicate the code, it would be harder to figure out what failed (since things would have the same line number - though using assertEqual() instead of assert would certainly help), and many of the tests are different enough that it wouldn't work anyway. In fact, while a lot of the tests are similar, many of them aren't in a pattern that would turn into a loop very well. Also, the unit testing model in D and how Phobos is set up isn't really intended to have the unit tests separate from the code. Doing so would be cumbersome and more bug prone. It's irritating enough to have to put the tests for some of the templated types after the type definition rather than in it. Managing the tests is _much_ easier when they're right next to the functions that they're testing. On top of that, many of them need private access, which they can't get in another module. So, while some could be moved out, I don't think that it would be a good idea, and many couldn't be moved out even if you wanted to. I grant you that there are a lot of unit tests, and ways probably could be found to trim them down, but they've helped a _lot_ in ensuring the code is correct, they have no effect whatsoever on user code since they'll be compiled-out, and I have found that maintaining them next to the functions that they're testing is actually easier to manage that it would be to try and split them out. So, I'm not particularly inclined to drastically change my tests. I think that the real question here is how good the API is. - Jonathan M Davis