in reply to Re^2: On comments
in thread On comments

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;
    }

[download]

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
    }

[download]

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

[download]

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.

Replies are listed 'Best First'.
Re^4: On comments
by pemungkah (Priest) on Dec 29, 2010 at 04:20 UTC
    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.)

[reply]

      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.
[reply]

In Section Meditations