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