About the Expressiveness of D

[Jonathan M Davis]

On Wednesday, April 03, 2013 08:53:09 Jacob Carlborg wrote:
> On 2013-04-03 05:03, Jonathan M Davis wrote:
> > I very much doubt that you could do that unless you specifically formatted
> > the code to take up as few lines as possible and didn't count the unit
> > tests or documentation in that line count. Otherwise, you couldn't do
> > anything even close to what std.datetime does in that few lines. Sure,
> > some functionality could be stripped, but you'd end up with something
> > that did a lot less if it were that small. The unit tests and
> > documentation do make it seem like a lot more code than it is, since they
> > take up well over half the file (probably 3/4), but you'd definitely lose
> > functionality with that few lines of code, and you'd end up with
> > something very poor IMHO if those 2000 lines included the documentation
> > and unit tests. You'd either end up with something that was very
> > bare-bones and/or something which was poorly tested, and given how easy
> > it is to screw up some of those date/time calculations, having only a few
> > tests would be a very bad idea.
> 
> Since he wrote "2000 lines for all functionality", I don't think he
> included unit tests or docs/comments.

That may be, but he does seem to have a habit of including the unit tests in 
the line count when he doesn't like how many lines of code a new piece of 
functionality takes up.

> > std.datetime's unit tests do need some refactoring (some of which I've
> > done, but there's still a fair bit of work to do there), which will
> > definitely reduce the number of LOC that they take up, but I don't agree
> > at all with considering the unit tests as part of the LOC of file when
> > discussing keeping LOC to a minimum. And while it's good to avoid
> > repetitive unit tests, I'd much rather have repetitive unit tests which
> > are thorough than short ones which aren't. I find your focus on trying to
> > keep unit tests to a minimum to be disturbing and likely to lead to
> > poorly tested code.
> > 
> > If anything, we need to be more thorough, not less. That doesn't mean that
> > the tests need to look like what std.datetime has (particularly since I
> > purposefully avoided loops and other more complicated constructs when I
> > wrote them originally in order to make them as simple and as far from
> > error-prone as possible), but unit tests need to be thorough, and while
> > we're getting better, Phobos' unit tests frequently aren't thorough
> > enough (particularly in std.range and std.algorithm when it comes to
> > testing a variety of range types). Too many of them just test a few cases
> > to make sure that the most obvious stuff works rather than making sure
> > they test corner cases and whatnot.
> > 
> > - Jonathan M Davis
> 
> I actually prefer to have repetitive unit tests and not using loops to
> make it clear what they actually do. Here's an example from our code
> base, in Ruby:
> 
> describe "Swedish" do
> subject { build(:address) { |a| a.country_id = Country::SWEDEN } }
> 
> it { should validate_postal_code(12345) }
> it { should validate_postal_code(85412) }
> 
> it { should_not validate_postal_code(123) }
> it { should_not validate_postal_code(123456) }
> 
> it { should_not validate_postal_code("05412") }
> it { should_not validate_postal_code("fooba") }
> end
> 
> describe "Finnish" do
> subject { build(:address) { |a| a.country_id = Country::FINLAND } }
> 
> it { should validate_postal_code(12345) }
> it { should validate_postal_code(12354) }
> it { should validate_postal_code(41588) }
> 
> it { should validate_postal_code("00123") }
> it { should validate_postal_code("01588") }
> it { should validate_postal_code("00000") }
> 
> it { should_not validate_postal_code(1234) }
> it { should_not validate_postal_code(123456) }
> it { should_not validate_postal_code("fooba") }
> end
> 
> It could be written less repetitive, like this:
> 
> postal_codes = {
> Country::SWEDEN => {
> valid: [12345, 85412],
> invalid: [123, 123456, "05412", "fooba"]
> },
> 
> Country::FINLAND => {
> valid: [12345, 12354, 41588],
> invalid: ["00123", "01588", "00000", 1234, 123456, "fooba"]
> }
> }
> 
> postal_codes.each do |country_id, postal_codes|
> describe c.english_name do
> subject { build(:address) { |a| a.country_id = country_id } }
> 
> postal_codes[:valid].each do |postal_code|
> it { should validate_postal_code(postal_code) }
> end
> 
> postal_codes[:invalid].each do |postal_code|
> it { should_not validate_postal_code(postal_code) }
> end
> end
> end
> 
> But I don't think that looks any better. I think it's much worse.

In general, I agree, because I think that straight-forward tests that avoid 
loops and the like are far less error-prone, and you need the tests to not be 
buggy. I don't want to have to test my test code to make sure that it works 
correctly.

However, I _do_ think that there's something to be said for refactoring the 
tests later (after the code supposedly fully works) to use loops and other 
more complicated constructs, because not only can that lead to more compact 
tests, but it also makes it much easier to make the tests more thorough 
(without taking many more lines of code). I just think that _starting out_ 
with the more complicated tests is not necessarily a good idea. Treating unit 
testing code as if it were the same is normal code doesn't make sense to me, 
if nothing else, because that would indicate that you're going to have to test 
your test code, since normal code is complicated enough to require testing. 
But Andrei and I have argued about this before, and I don't expect us to agree 
ever on it.

- Jonathan M Davis