www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - review for unittests

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jan 29 2011
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday 29 January 2011 11:46:55 Andrei Alexandrescu wrote:
 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.

I thought that I'd removed all of that. Apparently not.
 * 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.

You mean 80 lines of code in the examples or in the actual code? I'm fine with 80 columns for examples, but I'll go insane if we start requiring 80 columns for normal code. That starts getting restrictive and ugly _really_ fast. You're quickly forced to put the next part of a statement on the next line just because you're hitting the 80 character limit instead of properly lining things up, and it makes them much harder to read. As for the documentation, I made sure that the examples fit in the grey code boxes, and that's what I was aiming for. If that doesn't do it and restricting them to 80 columns does, then I'll do that.
 * 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'll make another pass then. I obviously missed some stuff. Feel free to point out spelling mistakes and the like. If they're there, it's pretty much a guarantee that it's because I missed them than because I didn't know better (though for some reason, I did get it in my head that it was generally-speaking rather than generally speaking; it does appear that you're correct however).
 * I appreciate the date-based examples but I'd rather stick with one
 focus at a time.

I used them so that I could give examples involving functions throwing exceptions without having to create types or functions in the examples which did that.
 * 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.

I confess that I hate having imports put together in one statement just like I hate declaring multiple variables on one line. I think that it harms readability and maintainability. But if you insist on putting imports on a single line in Phobos, then I can do that. As for the unit tests, what do you want me to do? They verify that the functions work correctly. They're thorough. That naturally makes them longer rather than shorter. You pretty much either get short unit tests or thorough unit tests. I tend to err on the side of thorough rather than short.
 * 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?

I'm all for finding ways to better organize the assertPred documentation. The general problem with it is that while it's quite simple to use, it has so many variations that it takes a fair bit of explaining. I'm not about to claim that it currently does it the best way that it could though.
 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.

I'll make another pass at at the documentation and see if I can hit a better balance between too little explanation and not enough. In fact, I think that I'll just merge it into std.exception (since you seemed to think that that would be the place to put it) and post what it would then look like once put into Phobos (since doing stuff like putting assertPred documentation in the module comment would affect the module as a whole and std.exception obviously already has stuff in it). I'll take care of that and get it up within the next few days. - Jonathan M Davis
Jan 29 2011
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 01/29/2011 09:48 PM, Jonathan M Davis wrote:
 You mean 80 lines of code in the examples or in the actual code? I'm fine with
80
 columns for examples, but I'll go insane if we start requiring 80 columns for
 normal code. That starts getting restrictive and ugly _really_ fast.

Yes, once you go beyond 80 columns.
 You're
 quickly forced to put the next part of a statement on the next line just
because
 you're hitting the 80 character limit instead of properly lining things up, and
 it makes them much harder to read.

When Gutenberg built the first press, the resolution and the coarseness of the paper forced him to make the sheets pretty large. Since then the resolution of printing has increased amazingly, but people quickly figured out that reading is seriously impaired if the layout has more than about ten words per row of text. So in the hundreds of years that people have had available for improving printing media, the ten words pattern has stayed put. It's a human constant. Such a measure definitely to code, too. 80 columns should be enough for writing and reading meaningful code. Lines that go significantly beyond that limit have an intrinsic readability problem.
 As for the documentation, I made sure that the examples fit in the grey code
 boxes, and that's what I was aiming for. If that doesn't do it and restricting
 them to 80 columns does, then I'll do that.

They don't fit on my screen.
 I'll make another pass at at the documentation and see if I can hit a better
 balance between too little explanation and not enough. In fact, I think that
 I'll just merge it into std.exception (since you seemed to think that that
would
 be the place to put it) and post what it would then look like once put into
 Phobos (since doing stuff like putting assertPred documentation in the module
 comment would affect the module as a whole and std.exception obviously already
 has stuff in it). I'll take care of that and get it up within the next few
days.

Yes, please integrate with std.exception. For the documentation: all you need is take your initial examples and make them compilable. Otherwise they were perfect. Thanks, Andrei
Jan 29 2011
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday 29 January 2011 23:08:49 Jonathan M Davis wrote:
 On Saturday 29 January 2011 22:10:29 Andrei Alexandrescu wrote:
 On 01/29/2011 09:48 PM, Jonathan M Davis wrote:
 You mean 80 lines of code in the examples or in the actual code? I'm
 fine with 80 columns for examples, but I'll go insane if we start
 requiring 80 columns for normal code. That starts getting restrictive
 and ugly _really_ fast.

Yes, once you go beyond 80 columns.
 You're
 quickly forced to put the next part of a statement on the next line
 just because you're hitting the 80 character limit instead of properly
 lining things up, and it makes them much harder to read.

When Gutenberg built the first press, the resolution and the coarseness of the paper forced him to make the sheets pretty large. Since then the resolution of printing has increased amazingly, but people quickly figured out that reading is seriously impaired if the layout has more than about ten words per row of text. So in the hundreds of years that people have had available for improving printing media, the ten words pattern has stayed put. It's a human constant. Such a measure definitely to code, too. 80 columns should be enough for writing and reading meaningful code. Lines that go significantly beyond that limit have an intrinsic readability problem.

I'm afraid that we're going to disagree on this one completely. Restricting code to 80 columns makes formatting a mess. You start doing things like breaking the next line based on how close it is to 80 columns rather than where it would make sense to break it based on the statement itself. Often, you're forced to put the remainder of the line on the next line simply indented somewhat more than the previous line rather than indenting it far enough to line up paretheses and function arguments. I much prefer lines that look like auto a = func(replace(str, "hello", "world"), 13 + max(b, c)); to auto a = func(replace(str, "hello","world"), 13 + max(b, c)); or auto a = func(replace(str, "hello","world"), 13 + max(b, c));

Hmmm. The HTML seems to have been lost when I sent it (I don't normally try and put HTML in e-mails, so I guess I screwed it up), so the code isn't formatted properly... The idea, at least, is that the first one lines up the arguments for each function call, the second one just indents the second line, and the third one lines up the beginning of the second line with what's on the right-hand side of the assignment. It'll probably look correct if you copy and paste it into a code editor. - Jonathan M Davis
Jan 29 2011
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 01/30/2011 01:08 AM, Jonathan M Davis wrote:
 I'm afraid that we're going to disagree on this one completely.
 Restricting code to 80 columns makes formatting a mess.

Apparently it's not a problem for all of my employers. All had such a standard. Facebook uses highlight-80+ with emacs and a commit hook to simply prevent lines longer than 80 characters from making it into the repository. So whereas I understand that you may not like the rule, I refute the implied objectivity of that argument. Code that fits in 80 columns is not a mess at least for some people.
 You start doing
 things like breaking the next line based on how close it is to 80
 columns rather than where it would make sense to break it based on the
 statement itself.

A statement that is so long is either over-indented or too long. In either case it should be breakable into bite-sized chunks. You need to break it to allow readers to read it easily, not to conform to an arbitrary rule.
 Often, you're forced to put the remainder of the line
 on the next line simply indented somewhat more than the previous line
 rather than indenting it far enough to line up paretheses and function
 arguments. I much prefer lines that look like


 auto a = func(replace(str,

 "hello",

 "world"),

 13 + max(b, c));


 to


 auto a = func(replace(str,

 "hello","world"), 13 + max(b, c));


 or


 auto a = func(replace(str,

 "hello","world"), 13 + max(b, c));



 Formatting becomes a _big_ problem when you force 80 character lines -
 _especially_ if you use descriptive names for functions and variables.

You'd be hard pressed to explain how quality code can arguably be written within 80 characters.
 And if you allow for greater than 80 character lines, then lines that
 would 90 or 100 characters and look just fine can be left on one line
 without having to worry about formatting problems at all.

I worry that I don't see the code, or that it comes off stilted at the beginning of the next line.
 The problem
 becomes particularly bad if you have code with multiple levels of scope.
 All of a sudden, your inner loop has to have all of its code take
 multiple lines just because it's indented far enough to be close to 80
 characters.

Then the function is poorly written and needs to be refactored. The problems are never with the 80 characters. I appreciate that you have a different perspective because you have been used to writing e.g. 100-columns code, and you believe you'd be incredibly constrained otherwise. Believe me: you will write better code in 80 lines.
 The _only_ reason I see to restrict the number of columns on a line like
 that is for printing code. If you make lines too long, they won't fit on
 paper. But they'll fit just fine on a computer screen at well beyond 80
 columns. With gvim, a line with 80 columns takes up less than a third of
 my screen - and that's with stuff like line numbers being shown. Unless
 you're specifically restricting the size of your edit windown to 80
 columns, I don't see why having more than 80 characters on a line would
 be a problem.

When you read a website, you don't maximize the browser (besides many sites forcibly limit the width of their text). Newspapers have columns to maximize parsing speed for their readers.
 And sure, you don't want lines getting to be 120 characters long and the
 like. There is a limit to how much it makes sense to put in a single
 statement.

Let's choose that limit to be 80 characters.
 However, I have routinely found that when I've been
 restricted to 80 column lines, code becomes far harder to format
 properly, statements which really should be on one line are forced to be
 on multiple lines because they're a bit passed 80 characters, and the
 code is harder to read.

I refute that because again there is overwhelming evidence to the contrary.
 I find that code is formatted much better when you're not strict about
 the length of lines and the like. You try and limit how long lines get,
 and you break them up on multiple lines when they start getting too
 long, but if you're strict about it, lines quickly become ill-formatted
 and harder to read.

I understand. I hope you also understand that your argument has only subjective basis, with you as the subject. You are literally only the second or third fellow coder to ever tell me such. Jonathan, I won't continue debating this. There is something to be said about picking one's fights, and that goes both ways. I will only say this. Phobos is a team effort. As such, there is a simple necessity to find ways to live and let live in relative harmony. This means conforming to conventions that are not the most comfortable to us (that includes me; my favorite bracing is the one in TDPL, but when I write Phobos code I use Walter's). Also, there is sometimes a need to conform to authority that we might sometimes not agree with; but as certain decisions are subjective, some de facto authority must make some decisions to simply keep the style somewhat consistent. It's unclear how authority comes about but it's reasonable to say that Walter's and my word have a somewhat larger weight because we wrote most of Phobos. I consider code that goes over 80 columns problematic, and I refactor it when I have the chance. This means that your commits mean more work for me. If you chose to use 80 columns, you'd be nice towards me and probably the other Phobosians, and you would get used to a widely used convention, which improves your adaptability to various workplaces. Andrei
Jan 29 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

On Saturday 29 January 2011 22:10:29 Andrei Alexandrescu wrote:
 On 01/29/2011 09:48 PM, Jonathan M Davis wrote:
 You mean 80 lines of code in the examples or in the actual code? I'm fine
 with 80 columns for examples, but I'll go insane if we start requiring
 80 columns for normal code. That starts getting restrictive and ugly
 _really_ fast.

Yes, once you go beyond 80 columns.
 You're
 quickly forced to put the next part of a statement on the next line just
 because you're hitting the 80 character limit instead of properly lining
 things up, and it makes them much harder to read.

When Gutenberg built the first press, the resolution and the coarseness of the paper forced him to make the sheets pretty large. Since then the resolution of printing has increased amazingly, but people quickly figured out that reading is seriously impaired if the layout has more than about ten words per row of text. So in the hundreds of years that people have had available for improving printing media, the ten words pattern has stayed put. It's a human constant. Such a measure definitely to code, too. 80 columns should be enough for writing and reading meaningful code. Lines that go significantly beyond that limit have an intrinsic readability problem.

I'm afraid that we're going to disagree on this one completely. Restricting code to 80 columns makes formatting a mess. You start doing things like breaking the next line based on how close it is to 80 columns rather than where it would make sense to break it based on the statement itself. Often, you're forced to put the remainder of the line on the next line simply indented somewhat more than the previous line rather than indenting it far enough to line up paretheses and function arguments. I much prefer lines that look like auto a = func(replace(str, "hello", "world"), 13 + max(b, c)); to auto a = func(replace(str, "hello","world"), 13 + max(b, c)); or auto a = func(replace(str, "hello","world"), 13 + max(b, c)); Formatting becomes a _big_ problem when you force 80 character lines - _especially_ if you use descriptive names for functions and variables. And if you allow for greater than 80 character lines, then lines that would 90 or 100 characters and look just fine can be left on one line without having to worry about formatting problems at all. The problem becomes particularly bad if you have code with multiple levels of scope. All of a sudden, your inner loop has to have all of its code take multiple lines just because it's indented far enough to be close to 80 characters. The _only_ reason I see to restrict the number of columns on a line like that is for printing code. If you make lines too long, they won't fit on paper. But they'll fit just fine on a computer screen at well beyond 80 columns. With gvim, a line with 80 columns takes up less than a third of my screen - and that's with stuff like line numbers being shown. Unless you're specifically restricting the size of your edit windown to 80 columns, I don't see why having more than 80 characters on a line would be a problem. And sure, you don't want lines getting to be 120 characters long and the like. There is a limit to how much it makes sense to put in a single statement. However, I have routinely found that when I've been restricted to 80 column lines, code becomes far harder to format properly, statements which really should be on one line are forced to be on multiple lines because they're a bit passed 80 characters, and the code is harder to read. I find that code is formatted much better when you're not strict about the length of lines and the like. You try and limit how long lines get, and you break them up on multiple lines when they start getting too long, but if you're strict about it, lines quickly become ill-formatted and harder to read. - Jonathan M Davis
Jan 29 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday 29 January 2011 23:45:24 Andrei Alexandrescu wrote:
 Jonathan, I won't continue debating this. There is something to be said
 about picking one's fights, and that goes both ways. I will only say this.
 
 Phobos is a team effort. As such, there is a simple necessity to find
 ways to live and let live in relative harmony. This means conforming to
 conventions that are not the most comfortable to us (that includes me;
 my favorite bracing is the one in TDPL, but when I write Phobos code I
 use Walter's). Also, there is sometimes a need to conform to authority
 that we might sometimes not agree with; but as certain decisions are
 subjective, some de facto authority must make some decisions to simply
 keep the style somewhat consistent. It's unclear how authority comes
 about but it's reasonable to say that Walter's and my word have a
 somewhat larger weight because we wrote most of Phobos.
 
 I consider code that goes over 80 columns problematic, and I refactor it
 when I have the chance. This means that your commits mean more work for
 me. If you chose to use 80 columns, you'd be nice towards me and
 probably the other Phobosians, and you would get used to a widely used
 convention, which improves your adaptability to various workplaces.

I do consider restricting code to 80 characters overly restrictive. I've hated it when I've had to program that way, and I think that the convention is outdated and that it should die. And most everyone I've ever discussed it with has generally agreed with me (though obviously not everyone). However, I _have_ had to code that way before (much as I fully believe that it tends to result in badly formatted code), and I can code that way in Phobos if that's what is insisted upon. It's just not what I do naturally, and if I have any say in a coding standard, I argue vehemently against it. So, if you insist upon it. Fine, I'll code that way for Phobos. I'm certainly not looking to cause trouble for anyone else. We're all stuck coding in ways that we don't like at least some of the time. But there's no question that being restricted to 80 columns is something that I consider to be very disagreeable. - Jonathan M Davis
Jan 30 2011
prev sibling parent "Robert Jacques" <sandford jhu.edu> writes:
On Sun, 30 Jan 2011 02:45:24 -0500, Andrei Alexandrescu  
<SeeWebsiteForEmail erdani.org> wrote:
 I understand. I hope you also understand that your argument has only  
 subjective basis, with you as the subject. You are literally only the  
 second or third fellow coder to ever tell me such.

Well, for your statistics, let me be the forth. :) But honestly, this is mostly due to the fact I'm generally using a high-res monitor which doesn't rotate, so my editor is about 150-chars wide but vertically narrow. (and that I will routinely break my own style, if it makes the code more comprehensible/readable to me). Of course, one of the nice things about a 150-char editor, is the 'two column' layout of 'code here; // Comment/doc over here'.
Jan 30 2011