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.
On Tue, 1 Jan 2019 at 16:22, Richard O'Keefe <rao...@gmail.com> wrote:
> On naming protocols: it would be nice to have clear guidance about
> when to use '-ion' and when to use '-ing'. In my own code I've been
> using '-ing' except for a few traditional categories like
> 'instance creation'. One reason for this was that I didn't want to
> keep on re-deciding "is it 'enumerating' or 'enumeration'".
>
> On Mon, 31 Dec 2018 at 22:04, Benoit St-Jean via Pharo-users <
> pharo-users@lists.pharo.org> wrote:
>
>> Besides, I forgot to add one comment : you should have guidelines for
>> protocol naming. It's not critical per se but I find it quite annoying
>> to look for stuff in "enumerating" when it's been put in "printing" !
>> loll
>>
>> On 2018-12-31 03:51, Stephane Ducasse wrote:
>> > Thanks! Thanks!
>> > I will go over it (now laying on my back :(( stupid muscles strike
>> back!)
>> >
>> > On Mon, Dec 31, 2018 at 2:18 AM Benoit St-Jean <bstj...@yahoo.com>
>> wrote:
>> >> On 2018-12-30 16:13, Stephane Ducasse wrote:
>> >>> Hello Fellow Pharoers
>> >>>
>> >>> I'm happy to announce a new little book to improve the culture around
>> Pharo.
>> >>> I will revise it in the future so you can feel free to send feedback
>> >>> and pull requests
>> >>> to https://github.com/SquareBracketAssociates/Booklet-PharoWithStyle
>> >>>
>> >>> Stef
>> >>>
>> >>> "The best way to predict the future is to invent" so I do it.
>> >> Stéphane,
>> >>
>> >> You'll find, attached, a revised PDF version with my comments.
>> >>
>> >>
>> >>
>> >> --
>> >> -----------------
>> >> Benoît St-Jean
>> >> Yahoo! Messenger: bstjean
>> >> Twitter: @BenLeChialeux
>> >> Pinterest: benoitstjean
>> >> Instagram: Chef_Benito
>> >> IRC: lamneth
>> >> Blogue: endormitoire.wordpress.com
>> >> "A standpoint is an intellectual horizon of radius zero". (A.
>> Einstein)
>> >>
>> --
>> -----------------
>> Benoît St-Jean
>> Yahoo! Messenger: bstjean
>> Twitter: @BenLeChialeux
>> Pinterest: benoitstjean
>> Instagram: Chef_Benito
>> IRC: lamneth
>> Blogue: endormitoire.wordpress.com
>> "A standpoint is an intellectual horizon of radius zero". (A. Einstein)
>>
>>
>>
