Mailing Lists: Apple Mailing Lists
Image of Mac OS face in stamp
 
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [OT] Commenting source code (was Re: Problems with Controls Manager)

[Part 2 of 2: continuing from part 1]

At 22:02 -0400 29-08-2003, Laurence Harris wrote:
On 8/29/03 9:21 PM, Camillo Lugaresi didst favor us with:
> The pitfalls of excessive self-documentation are the same as those of
excessive commenting. The only difference is that your text will be
mostly blue instead of mostly red (or whatever color). :)

I wouldn't use something like that myself unless I had a pretty good reason
to, but I wouldn't rule out the possibility.
>
> I think your earlier attempt was a better example of practical
self-documenting code:

At 15:57 -0400 27-08-2003, Laurence Harris wrote:
const SInt16 windowWidth = intRight - intLeft,
windowHeight = intBottom - intTop,
kScrollbarWidth = 15;

SizeControl( control, windowWidth, windowHeight - kScrollbarWidth );

Sure, but this is a tiny amount of code. If it were larger and/or there were
some tedious detail I don't need to read through to get an overall picture
of the function containing this code were doing, I might put it in a
function.

Well, of course. In a different situation, things would be different. That's tautological. =)

One of the reasons to factor stuff out into smaller, more narrowly
focused functions is so you don't get bogged down in details when trying to
read the function that calls those helper functions.

I fully agree. I merely said that your example seemed inappropriate, not that your whole thesis was flawed. So, you don't need to repeat that thesis to me.
I recognize the merits of self-documenting code.

> Of course it's not perfect, but if someone gets puzzled because
you're using a couple of local variables, it's his job that should be
changed, not your code. ;)

Well, that's my perspective. Like I said, the few comments I write are not
intended to be lessons in how to write Mac software. ;-)

In fact, if you use enough of these kinds of functions the logic of your
code can become extremely easy to follow just from reading a sequence of
well-named functions:

CreateWidgetWindow( &widgetWindow );

CreateWidgetControlToDisplayContent( widgetWindow , &widgetControl );

>> SizeControlToFillWindowLeavingSpaceForHorizScrollbar( widgetControl,
widgetWindow );
>>
CreateHorizontolScrollbarAtBottomOfWindow( widgetWindow, &scrollbar );

get(&clue); /* just use a NIB ;) */


get(&clue); // This was just an example

Ah, but it was an especially bad example of self-documenting code.
It is, however, a good example for a point I was planning to make, so I thank you for keeping my attention on it. :)

What happens if one needs to add a header to the window? A programmer might go to the place where the control size is calculated, inside the body of SizeControlToFillWindowLeavingSpaceForHorizScrollbar(), and change it to take the header into account.

But what if another programmer (or the same, some time later) wants to create another control in a different window, which has no header, just a horizontal scrollbar? He might try to reuse the SizeControlToFillWindowLeavingSpaceForHorizScrollbar() function, and he'll be puzzled when he finds that the control does not extend to the top edge of the window. He might think it is a bug, and change the code again, which of course will cause problems in the other window.

The original programmer should have changed the name of the function to SizeControlToFillWindowLeavingSpaceForHorizScrollbarAndHeader() when he changed it to leave room for the header (I say change the name, and not make another, because it was being used in a single place at that time, but it's not really important). But perhaps he was in a hurry and he thought such a minor change did not have to be mentioned in the function name; or perhaps he simply forgot.

As you can see, this is the same synchronization problem you pointed out for comments.

Compare the following code blocks:

A)
// Size control to fill window, leaving space for horizontal scrollbar
Rect windowBounds;

GetPortBounds( GetWindowPort( window ), &windowBounds );
SizeControl( control, WIDTH( windowBounds ), HEIGHT( windowBounds ) - kScrollbarWidth );

B)
SizeControlToFillWindowLeavingSpaceForHorizScrollbar( ControlRef control, WindowRef window )
{
Rect windowBounds;

GetPortBounds( GetWindowPort( window ), &windowBounds );
SizeControl( control, WIDTH( windowBounds ), HEIGHT( windowBounds ) - kScrollbarWidth );
}

They have the same problems. Both waste time describing two really obvious lines of code. Both descriptions can go out of sync with what the code really does. Both use macros for the width and height of a rectangle, where they should really use inline functions. =)
As I said, excessive self-documentation and factoring leads to the exact same problems as excessive and thoughtless commenting.
In fact, in this example the self-documenting version is a bit worse, since the description is less readable, takes more time to update since it must also be changed in the body of the caller, and the whole thing took longer to write. ;)

Again, the obligatory disclaimer: I am not opposed to self-documenting code, I'm just saying it must not be abused, else you'll end up reproducing all the same problems you had with comments. :P

Camillo
_______________________________________________
carbon-development mailing list | email@hidden
Help/Unsubscribe/Archives: http://www.lists.apple.com/mailman/listinfo/carbon-development
Do not post admin requests to the list. They will be ignored.


Visit the Apple Store online or at retail locations.
1-800-MY-APPLE

Contact Apple | Terms of Use | Privacy Policy

Copyright © 2007 Apple Inc. All rights reserved.