All the Perl that's Practical to Extract and Report

Skimmable Code & end-of-block comments

schwern on 2008-06-17T15:53:23

I did my "Skimmable Code" talk yesterday and from the response it seems to have gone over very well. One thing that caught me entirely off guard was the response to the suggestion that end-of-block comments are a red flag for skimability and, once the block has been refactored to be shorter, should be removed.

while( $something > 42 ) {
    ...some code...
    ...and stuff...
} # while $something > 42


Usually any stylistic assertion gets at least one unhappy response from the audience. This one had it's own cheering section. I was a bit taken off guard by that and didn't have a good response. The argument was that when you can't see the condition, and no matter how short the block sometimes you can't -- maybe you're looking at code just after the block. These comments help put information in front of your face that otherwise you'd have to page up and down to see. And that's what skimmable code is about, putting all the relevant information in front of your face so you don't have to remember it.

End-of-block comments surely are a red flag. Here's an example from WWW::Mechanize which uses end-of-block comments habitually.

for my $value ( @_ ) {
    # Handle type/value pairs an arrayref
    if ( ref $value eq 'ARRAY' ) {
        my ( $type, $value ) = @$value;
        while ( my $input = shift @inputs ) {
            next if $input->type eq 'hidden';
            if ( $input->type eq $type ) {
                $input->value( $value );
                $num_set++;
                last;
            }
        } # while
    }
    # by default, it's a value
    else {
        while ( my $input = shift @inputs ) {
            next if $input->type eq 'hidden';
            $input->value( $value );
            $num_set++;
            last;
        } # while
    }
} # for


They're not used here because the blocks are too long. They're used because nested structures, especially nested loops, are confusing. Even with good indentation habits. This leads to a need to comment which block is being closed. It's not even showing the conditional. They're a symptom of complexity. The comments should say to you "maybe this can be simpler".

I still recommend deleting them once the code has been refactored to be more skimmable. Why? It's more to filter out while skimming. It's a comment about *what* the code is doing. It's repeating what the code already says rendering it redundant. It's something you can ignore and thus just clutters up the screen. A good comment adds more information, saying why the code is there or explaining a complicated bit.

Straying into stylistic opinion, the comment makes it harder for me to see the closing brace. Rather than having a clean line with one character on it, there's a fuzz of comments breaking up the visual.

End-of-scope comments violate the DRY principle, you're repeating code. As such, it becomes a maintenance hassle. The comment is likely to not get updated as the conditional changes, especially if the comment repeats the whole condition, and you wind up with a lying comment. It's a bad comment, it shouldn't be there.

But, the original desire is valid. It's nice to be able to see the condition from the bottom of a scope. If it's rote code inspection, monkey work, let your monkey do it. This is something your editor should be doing for you. It's 2008. If I can't have my jetpack I can at least have an editor that can show me where a block starts. Emacs does this when you type the closing brace. I'm not sure how to get it to do it on-hover, but I'm sure someone will respond with the appropriate magic. Then, 5 minutes later some clever vi code will be forthcoming. :)

Appropriate Magic(tm)

jplindstrom on 2008-06-18T00:23:50

Put this in your .emacs

(show-paren-mode)

Re:Appropriate Magic(tm)

schwern on 2008-06-18T18:39:33

That doesn't appear to do anything if the other paren if off the screen. Maybe I don't have some other magic set. Does it work for you?

Re:Appropriate Magic(tm)

jplindstrom on 2008-06-19T01:12:59

Um, from reading your text, "If I can't have my jetpack I can at least have an editor that can show me where a block starts. Emacs does this when you type the closing brace. I'm not sure how to get it to do it on-hover", I understood that as "highlight the matching curly" whenever the cursor is on a { or }

(similar to "mouse hover", but Emacs isn't very mouse oriented in general...)

Did you mean something else?

Re:Appropriate Magic(tm)

schwern on 2008-07-06T10:38:37

Um, from reading your text, "If I can't have my jetpack I can at least have an editor that can show me where a block starts. Emacs does this when you type the closing brace. I'm not sure how to get it to do it on-hover", I understood that as "highlight the matching curly" whenever the cursor is on a { or }

It's not very useful if the matching curly has scrolled off the top of the screen, ie. I can't see it. Setting show-paren-style to "parenthesis" it will highlight the other paren, which is off screen. If I use "expression" it will highlight the whole block, but the condition still lies off screen and out of sight.

What I really want is it to say "Matches if( foo ) {", like it does when you type a closing brace. Show me the first line of the block so I can see the condition, the important bit.

(similar to "mouse hover", but Emacs isn't very mouse oriented in general...)

This is why I use Aquamacs. :) But really I just mean when the cursor is over a paren or brace rather then when it's typed.

.vimrc

mattk on 2008-06-18T04:57:24

In vim, the % key lets you bounce between chars, and putting 'set showmatch' in your .vimrc will briefly highlight the opening brace when you type a closing one.

An idea for the IDEs..

patspam on 2008-06-18T06:18:41

This is something that an intelligent graphical IDE could show as a tool-tip when you hover the mouse over the closing brace. That way you don't need to repeat yourself in code, and you can still find out what the brace corresponds to without scrolling.

Patrick

Re:An idea for the IDEs..

jplindstrom on 2008-06-18T12:36:25

Or you could display it permanently at the end of the block, making it look like a tooltip or something, but not as a comment in the source code. (in Emacs, an overlay)

I'm note sure I'd appreciate it though, it may be too noisy.

Maybe it would be useful if it was only displayed whenever the block is longer than n lines, or whenever the other side of the block isn't visible.

This may be one of those things that sound good/bad, but you won't really know how it pans out until you try it :)

some ide's do this

WebDragon on 2008-06-18T16:54:27

Geany for example, shows a sidebar brace wrapping any blocks that occur in your code, such as subroutine functions, comment blocks, if statements, etc.

I don't use geany myself but I can see how such a thing would be useful to coders.

Now where's that vim plugin? :-) ...of course one can use folding in Vim as well.

Re: folding

Eric Wilhelm on 2008-09-07T05:19:51

... of course one can use folding in Vim as well.

If you're doing it right, the block is usually short enough that folding it isn't worthwhile.

Thankyou

Mutant321 on 2008-06-18T20:54:02

End of scope comments are a pet hate of mine, for all the reasons you've given. And it's one of those things that people seem to cling to, as if it's a best practice like TDD.

I hate clutter of any sort in code... which is a shame since I have to use Java for my next project...