I'm not sure that the advice on method comments is as good as it could be.
Consider Point >>
setX: xValue setY: yValue
x := xValue.
y := yValue
It is pretty obvious what this does, and if I'm reading chapter 4 correctly
the advice is that it doesn't need a comment. But it does: why DOESN'T this
method check that xValue and yValue are numbers, something which is assumed
by practically every method on the instance side? In short, why does
(Point x: nil y: nil) r
fail when the point is USED instead of when the point is CREATED?
*Somewhere*, in the class comment, the class-side #x:y: method, or this
method, this ought to be explained.
In my own code, every class has an (instance-side) #invariant method which
I can call from the debugger. Even if it is just ^super invariant, at least
that's explicit documentation. I used to write comments, and then I realised
that code I could call was even better. That's something useful to say about
comments too.
"Method comments should contain sufficient information for a user to know
exactly how to use the method, what the method does including any side ef-
fects, and what it answers without having to look at the source code."
This is advice that almost none of the code in Pharo follows.
What the method *does* and *answers* can be determined by reading the code;
in Smalltalk you are much more likely to have (decompiled) source code
without comments than comments without source code. What the method is
*for* is the thing that you cannot see in the code.
To get an example, I did (String allSelectors atRandom), getting
#copyWithoutAll:. That has only one definition, in Collection:
copyWithoutAll: aCollection
"Answer a copy of the receiver that does not contain any elements
equal to those in aCollection."
^ self reject: [:each | aCollection includes: each]
The comment simply paraphrases the rather simple code.
It does NOT explain how to use the method: when (if ever) is it better
to pass a set than a string? Above all, why is it a good idea for
aCollection to be in charge of which elements of the receiver count as
equal to something in aCollection instead of the receiver being in
charge? Consider
|c a|
c := IdentitySet with: 0.0.
a := Array with: c anyOne * 1.0.
{ c copyWithoutAll: a. a copyWithoutAll: c }
Why does the >Identity<Set use #= for the decision while the Array
uses #==? Why is the code not
^self select: [:it |
aCollection noneSatisfy: [:each | it = each]]
? It is the code that is NOT there that needs explaining most.
I am not blaming the Pharo team here. They have other maddened grizzly bears
to stun, and did not invent this particular method. Just by sheer dumb luck,
the random number generator turned up a method that has always annoyed me for
obviously but inexplicably doing something seriously weird.
Oh, there's another important thing about that particular comment.
It is wrong. It refers to "elements equal to", which leads the reader
to assume that the same notion of equality in every case. It should read
something like
"Answer a copy of the receiver that does not contain any element
equivalent to an element of aCollection, with equivalence defined
by aCollection and NOT by the receiver."
There should be some advice about wrong or misleading comments being worse
than none.
PS: Benoit St-Jean did not catch all the typos.
Locked
bstj_revised_withStyle-wip.pdf (286K)