review for unittests
Andrei Alexandrescu
SeeWebsiteForEmail at erdani.org
Sat Jan 29 11:46:55 PST 2011
I made one more pass through the unittests library and it looks fine,
but I still find it wanting. Overall:
* From too few examples now it has too many! The fine line is somewhere
in between (I'd say on the short side). I complained about the initial
documentation because the example were meaningless (e.g. they used names
that weren't defined. The current iteration of the library gives so many
examples it's patronizing. Yes, if you give the list of operators
supported, you don't need to give an example of each! We're not
oligophrenes over here.
* To add insult to injury, examples have a double-spacing that is a bit
much.
* The code has a lot of lines beyond 80 characters. 80 characters should
be plenty to write good code, and makes for readable code that doesn't
need to occupy one's entire screen. I wish I could somehow convince
everybody to not complain, not debate, and not show me the old style
guide. Let's just write 80-columns code. That includes documentation.
Please. Let's.
* Even examples are sometimes too wide - some don't fit in my browser
unless I enlarge it considerably.
* I don't mean to belittle the effort, but the documentation needs one
more pass for typos etc. I wouldn't mention this if I hadn't noticed
Jonathan is otherwise a perfect speller and a good writer. So, it's
"Generally speaking" not "Generally-speaking", "given set" not "give
set"... and that's just the first line. Eliminate all flowery: there's
no need for "generally speaking" and "in essence" "particularly" etc.
etc. etc.
* For inline code use $(D ) not anything else
* I appreciate the date-based examples but I'd rather stick with one
focus at a time.
* There's got to be too much of a good thing. The massive unittests, the
lavish waste of vertical space (import with commas, anyone?) and the
documentation make adding four simple concepts a 1887 lines deal. I
guess that's borderline okay but I am increasingly hawkish about massive
additions of small functionality to Phobos.
* The documentation for the mock-up assertPred could be moved up to the
module documentation, which has the advantage that it avoids any
self-explaining: "There is further documentation for each version below
(this particular version of the function doesn't actually exist - it's
here so that there's a good spot to put documentation for the function
as a whole)." Why would anyone need to absorb that kind of information
in order to use four simple concepts?
My vote is in favor to adopting this library, contingent upon (a) making
a spelling and correctness pass and (b) giving the documentation a
thorough haircut. The rest of my comments are optional as they could be
effected later.
Andrei
More information about the Digitalmars-d
mailing list