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.
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.
--
/Jacob Carlborg
