Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl Monk, Perl Meditation
 
PerlMonks  

Re^2: On comments

by pemungkah (Priest)
on Dec 28, 2010 at 10:21 UTC ( #879411=note: print w/replies, xml ) Need Help??


in reply to Re: On comments
in thread On comments

Please head on over to perl5db.pl, and fix that up so it doesn't need comments. I'll wait. :)

That was a big project - about a year - and all I did was comment the code. Why? Because nobody other than a very few people understood the debugger. Reading that code with no comments is sort of like hearing half a phone conversation, as the debugger is really just the Perl side of the execution-time hooks that make debugging possible. The comments clarify the things that are happening elsewhere that are important to the the debugger code that's just been called from someplace else. That contextual info is important - and it can't be in the code; it has to be in the comments. Yes, you could switch back and forth between the debugger source and the Perl source, but you'd have to understand the Perl core pretty well. With comments, you can just read and understand in much less time with much less effort.

Compare the 5.004 debugger to the 5.8.8 one to see the difference between the uncommented source and the commented. I think it was worth doing, as (among other things) Devel::ebug and the beginnings of the debugger tests in the Perl core tests came out of that. I firmly believe it was because the stuff you should "just get from the code" was finally laid out, explained, and made possible to follow.

I also believe that nearly anyone who's basically familiar with Perl can now read the debugger source and actually get something out of it other than a headache, even if it's a resolution to never ever write code like that.

Comments, as I mentioned, are not meant to tell you what the code can tell you itself; they are meant to tell you what decisions went into putting that code there in the first place, and the context in which these decisions made sense. As in the kernel bug noted, the reason for the decision is sometimes more important and considerably more complex than the code itself is. When a few characters make the difference between a root exploit and secure code, put in the comment!

Writing comments is writing; if the comments are good, then they communicate an arc of logic from point start to finish. The combination of the comments and the code should tell you more than either could alone. If they don't then the comments are wasted space - but should be condensed, corrected, and made to communicate, not simply discarded!

The comments and code conflict? There are several possible reasons, but one important possibility is that there really was a disconnect between the desire and the implementation, and that the code is actually wrong. It may be functioning and passing its tests, but it may nonetheless not be what was actually intended. It may be very hard to find out what was really wanted - and the code and comments don't help - but the fact that they conflict communicates something important that the code alone would not and cannot. If you simply strip the comments and then look at the code, you've thrown away data. Never a good idea when you're looking at code you don't know well (meaning that you know what was wanted, what it's doing, and why).

I honestly find an approach that says "never" comment or "always" delete comments suspect; it's as if the programmer had decided that they didn't like else clauses. Sure, you can write code that's functionally equivalent without them, but you've made a choice that deliberately makes the code's primary job - communicating to the next programmer - more difficult, and I think that's a bad choice.

Comments are as integral a part of the language, and if you've decided never to use them, you really should think about that, and be sure it's not (for instance) compensating for feelings of insecurity. "I'm so good I don't need comments! Really I am!"

I prefer to know that I need comments some places, and I make sure I put them in at those places. I don't think I'm not as good because I comment; I think I'm more considerate and better cognizant of my abilities by doing so. Maybe the next programmer really will be so good he or she doesn't need comments. Let's hope the one after the decommenter is as good as the decommenter thinks he or she is.

Know your audience, and be honest and pragmatic. Not every programmer is going to be as brilliant as every other. You're not going to be as brilliant every day. Make sure the average programmer (and you) will be able to read the code with the comments and get as much out of it as the brilliant one can without the comments. This is eminently possible, and worth it, in my opinion.

Replies are listed 'Best First'.
Re^3: On comments
by BrowserUk (Pope) on Dec 28, 2010 at 11:41 UTC

    Sorry, but bad comments do not make up for bad code.

    All of this:

    The following variables are altered by C<DB::eval()> during its execut +ion. They are "stacked" via C<local()>, enabling recursive calls to C<DB::eval() +>. =over 4 =item C<@res> - used to capture output from actual C<eval>. =item C<$otrace> - saved value of C<$trace>. =item C<$osingle> - saved value of C<$single>. =item C<$od> - saved value of C<$^D>. =item C<$saved[0]> - saved value of C<$@>. =item $\ - for output of C<$@> if there is an evaluation error. =back =head3 The problem of lexicals The context of C<DB::eval()> presents us with some problems. Obviously +, we want to be 'sandboxed' away from the debugger's internals when we d +o the eval, but we need some way to control how punctuation variables an +d debugger globals are used. We can't use local, because the code inside C<DB::eval> can see locali +zed variables; and we can't use C<my> either for the same reason. The code in this routine compromises and uses C<my>. After this routine is over, we don't have user code executing in the d +ebugger's context, so we can use C<my> freely. =cut ############################################## Begin lexical danger zo +ne # 'my' variables used here could leak into (that is, be visible in) # the context that the code being evaluated is executing in. This mean +s that # the code could modify the debugger's variables. # # Fiddling with the debugger's context could be Bad. We insulate thing +s as # much as we can. sub eval { # 'my' would make it visible from user code # but so does local! --tchrist # Remember: this localizes @DB::res, not @main::res. local @res; { # Try to keep the user code from messing with us. Save these +so that # even if the eval'ed code changes them, we can put them back +again. # Needed because the user could refer directly to the debugger +'s # package globals (and any 'my' variables in this containing s +cope) # inside the eval(), and we want to try to stay safe. local $otrace = $trace; local $osingle = $single; local $od = $^D; # Untaint the incoming eval() argument. { ($evalarg) = $evalarg =~ /(.*)/s; } # $usercontext built in DB::DB near the comment # "set up the context for DB::eval ..." # Evaluate and save any results. @res = eval "$usercontext $evalarg;\n"; # '\n' for nice recur +sive debug # Restore those old values. $trace = $otrace; $single = $osingle; $^D = $od; }

    Can be replaced by:

    { local( $trace, $single, $^D ); ($evalarg) = $evalarg =~ /(.*)/s; # Untaint # $usercontext built in DB::DB @res = eval "$usercontext $evalarg;\n"; # '\n' for nice recur +sive debug }

    Not only is it shorter and clearer, it is safer. As tchrist points out in the above code:

    # 'my' would make it visible from user code # but so does local! --tchrist

    You've gone to great verbosity to explain how the variables: $otrace, $osingle, $od, are used to prevent the user code from messing with the debugger's internal state. Completely missing the fact that by storing copies in those localised globals, it is just as likely that the user code will mess with those variables as the originals. And if they do, the code will be restoring the messed with values over the top of the untouched original values.

    Whereas if you simply localise the original globals, they'll be restored when the block exits regardless of what the user does. And the need for all the code-concealing and confusing comments just goes away.

    I can see from what you've done to perl5db.pl that we'll never agree. That's fine. Some people like marmite, some don't.

    But, the last four paras of your post above show me that you've either not read Programming *is* much more than "just writing code"., or you have and still think you can justify your position by concluding that those with the opposed viewpoint are either too lazy or too arrogant to comment. And that is just plain not the case.

    I rarely comment because I've found over the years that the vast majority of comments tell me nothing more than the code does. But worse, as above, the comments attempt to persuade me stuff that is just flat out wrong.

    Even your point that "Not every programmer is going to be as brilliant as every other." totally misreads that reasoning. For example, any programmer unfamiliar with local--which is a larger portion of modern Perl programmers--will, from reading your comments above, get entirely the wrong idea about what it does and how it works. And will go away with entirely the wrong impression because of those bad comments.

    If however, there were no comments, they would have had to have gone to the documentation, and read up about the local keyword and learnt what it really does.

    Like I said. Bad comment are worse than no comments. And most comments are bad. Even those by people who think they write good ones.


    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.
      I have read Programming *is* much more than "just writing code". and it's clear that we come from diametrically opposite points about comments, which is fine. If we were working on the same team we'd need to find a compromise. I'm going to use the rhetorical tool I mentioned a while back in the node about "agressive argument" (Re: What is "aggressive" argument?) to defuse the discussion, which feels a little heated to me: since it is at least partially a matter of taste, this is one of those arguments that can't be definitively "finished" or "won", only abandoned - and I think that this is probably the proper point for me to do that. This is not saying "I'm right" or "you're wrong", just "we're not going to agree about this, I think".

      It's possible that I've simply not clearly stated the assumption I'm working from: comments don't actually affect the logic of the program, Smart::Comments aside - but they do affect the logic of the programmer, and what he or she will decide to do.

      Intentions, political considerations, external dependencies, and other similar factors that have nothing to do with the local code that's on the page (but everything to do with the context in which it was developed and in which it must run) are important, and that not making sure they are known at the exact point where further decisions need to be made in the source code will increase the chances of those decisions being wrong, perhaps catastrophically so.

      I personally think that comments are the best way to do that: right at the code that was affected by the external circumstance (some other obscure code called it, in the case of the debugger, or that variable really does have to be zero or we have a remote root exploit in the Linux kernel source). Is there a better way to communicate that kind of thing at the point of decision: at the place in the code that might be changed without a signpost that doing so would be Bad? Remember, I'm not asking you to make the code clearer; I'm asking you to make the context clearer. What's the best way to do that?

      I think we agree that this non-local-code context is important, and difficult to communicate effectively. I would be very interested to hear what ways you use to get all that messy not-code stuff across to "the next victim".

      (Edit: spoiler that shows the comments from perl5db.pl wasn't working for me, but is now...go figure. Minor wording changes for clarity.)

        There is no "heat" here. This debate will go on--for both of us, and every programmer--long after we've stopped talking. We, just as all those on opposing sides of the debate will simply have to agree to differ.

        Remember, I'm not asking you to make the code clearer; I'm asking you to make the context clearer. What's the best way to do that?

        That question presupposes that what you've said in the preceding two paragraphs is important to the ongoing maintenance of the code. And that is where we will have to agree to differ.

        I don't want to make too much of your example, as your efforts were obviously a labour of love, but IMO there are about 7 or 8 significant comments in there. But, due to (what I consider to be), the morass of unnecessary, irrelevant or downright misleading thoughts, feelings, opinions and speculations--not just duplicating what is clear from the code, but often triplicating or even quadruplicating it once you consider the block comments; inline comments and interleaved POD documentation--it is neigh impossible to pick out those important ones from that morass.

        Were I ever to attempt to modify that program--and I might because although I rarely use the debugger, it is useful now and again, but is completely broken in 5.10 on my platform--I would strip all the comments so that I could see the code. I would retain an original and feed any changes I made back into it if I was considering submitting a patch.

        But, for me, the very essence of the coders art is the code. And, IMO, trying to read the code when it is so thoroughly concealed in untested & unverified--indeed, untestable & unverifiable--prose, is like trying to study the stars from the middle of a major city. There is just so much extraneous noise that it completely swamps what is actually important.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://879411]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others rifling through the Monastery: (5)
As of 2020-06-05 23:09 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Do you really want to know if there is extraterrestrial life?



    Results (41 votes). Check out past polls.

    Notices?