Too much of a good thing?



  • I know comments are a good thing. I know JavaDoc is good. But I wonder, where would you personally draw the line between well-document code and WTF document code? I'm asking because I recently stumbled across a class of ours which had been refactored by a new developer. Here's one method:

        /**
         * Set whether or not we are appending to an already existing file
         *
         * @param append Whether are appending or not
         */
        public void setAppend(boolean append) {
            // Set instance variable append to the value passed in
            this.append = append;
            // If we are appending to a file...
            if (append) {
                // ...we do not want column titles written to the file
                setNoTitles(true);
            }
        }

    Now, it's not small methods like that, this is throughout the code. Virtually every other line has a long description of exactly what it does, down to things like:

    // call method to set the path
    setServerPath(file);
    Log.debug(this, "Set server path to: "+file);

    <FONT face="Times New Roman">Is it too much? Is it just right? Could this be the solution to all our problems? Or does it make the code unreadable?</FONT>



  • @phelyan said:

    <font face="Times New Roman">Is it too much? Is it just right? Could this be the solution to all our problems? Or does it make the code unreadable?</font>





    I presume you are beeing either rethorical or sarcastic :D If a
    programmer doesn't even understand the proper use of comments, what
    about the other, more complex programming constructs?




  • I do not understand what the comments mean, so I consider them bad, and little more that 'warm bytes'.



    My eye also tends to start twitching a bit when I see the use of 'we' in a comment.



  • Thing is, most of the comments just describe exactly what happens in
    the code. No further remarks as to the why. I know what an if statement
    does and I can read the method names. 'Warm bytes', but Boss loves 'em.



  • The question is wrong. Comments are not a good thing; they’re a tool that can be used for a good purpose. In the example you show, they’re not.

    Besides the fact that “what”/”how” comments as opposed to “why” comments are actively harmful. Comments have to be kept in synch with the code. But they are not executed and they cannot be debugged.

    A good codebase has

    • very well-chosen identifiers
    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    Tons of comments is a red flag in and of itself.



  • @Aristotle Pagaltzis said:

    The question is wrong. Comments are not a good thing; they’re a tool that can be used for a good purpose. In the example you show, they’re not.

    Besides the fact that “what”/”how” comments as opposed to “why” comments are actively harmful. Comments have to be kept in synch with the code. But they are not executed and they cannot be debugged.

    A good codebase has

    • very well-chosen identifiers
    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    Tons of comments is a red flag in and of itself.

    As a rule of thumb, I always stop writing comments as soon as 'we' starts to appear.

    Drak



  • @Drak said:

    @Aristotle Pagaltzis said:

    The question is wrong. Comments are not a good thing; they’re a tool
    that can be used for a good purpose. In the example you show, they’re
    not.

    Besides the fact that “what”/”how” comments as opposed to “why” comments are actively harmful. Comments have to be kept in synch with the code. But they are not executed and they cannot be debugged.

    A good codebase has

    • very well-chosen identifiers
    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    Tons of comments is a red flag in and of itself.

    As a rule of thumb, I always stop writing comments as soon as 'we' starts to appear.

    Drak



    Could you tell me why that is?  I've always been a bit confused as to when my comments go from helpful to annoying.  In college, my professors would always tell me when my comments were "wrong," but they'd rarely tell me what was "right."  Thus I find that on the job, I just kind of flip-flop between the two extremes of hardly any comments and a comment for almost every line.


  • @UncleMidriff said:



    Could you tell me why that is?  I've always been a bit confused as to when my comments go from helpful to annoying.  In college, my professors would always tell me when my comments were "wrong," but they'd rarely tell me what was "right."  Thus I find that on the job, I just kind of flip-flop between the two extremes of hardly any comments and a comment for almost every line.

    When your comment becomes more of a novel than a set of one-liners, it's time to stop, rewind and remove all the nonsense. I must admit though that sometimes I do not comment a function at all (except for the header comment block), because it should be obvious what it does and how it does it. The 'why' isn't always important anyway.

    Drak



  • I was taught explicitly several times to make a best effort to comment every line.


    I tend to agree. Around 3am semi-colons get hard to read. Frankly when
    all is said and done there is no such thing as readable code. English
    is always good to have.



  • @sadmac said:

    I was taught explicitly several times to make a best effort to comment every line.




    No.



    Absolutely not.



    No.



    Your teachers are wrong. It's not bullshit to comment 'often' but it's
    certainly bullshit to comment 'every line'. For functions in a library,
    I often comment the in and out, just above the function definition.
    Other than that, in complex routines, I comment to let people know what
    a specific couple of lines do.



    Comments are there to prevent people from asking 'wtf does this do?'
    You don't comment for yourself, you comment for others and your future
    self.



    @sadmac said:
    Around 3am semi-colons get hard to read.




    But you have to understand the code to add proper comments, so it's
    like a catch-22. You want to add comments to better understand the code
    at 3am, but to add comments you have to understand the code, which
    makes adding comments redundant.



    Around 3am, you'll write bad code anyway, so commenting [i]everything[/i] will only increase the kludge.



  • @sadmac said:

    Frankly when all is said and done there is no such thing as readable code. English is always good to have.

    I agree with the first sentence, but I don’t know how you get from there to the second. Having English would be nice if that was all that was there; if it’s in addition to what’s got to be there, that spells trouble.

    When the code and the comments disagree, both are probably wrong.Norman Schryer

    Remember that you can only debug code – not prose…



  • When I write code, I rarely comment the majority of the code.  I just write a comment for the function as to its purpose, the prerequisites of the function and the expected output.  I only comment the actual code when I need to remind myself why I did some "magical" programming, or where a bug/catch 22 may occur in certain situations.  It is absolutly absurd to code each and every line or every other line. Commenting should be kept to a minimum and where needed.



  • @UncleMidriff said:

    @Drak said:
    @Aristotle Pagaltzis said:

    The question is wrong. Comments are not a good thing; they’re a tool
    that can be used for a good purpose. In the example you show, they’re
    not.

    Besides the fact that “what”/”how” comments as opposed to “why” comments are actively harmful. Comments have to be kept in synch with the code. But they are not executed and they cannot be debugged.

    A good codebase has

    • very well-chosen identifiers
    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    Tons of comments is a red flag in and of itself.

    As a rule of thumb, I always stop writing comments as soon as 'we' starts to appear.

    Drak



    Could you tell me why that is?  I've always been a bit confused as to when my comments go from helpful to annoying.  In college, my professors would always tell me when my comments were "wrong," but they'd rarely tell me what was "right."  Thus I find that on the job, I just kind of flip-flop between the two extremes of hardly any comments and a comment for almost every line.


    I'm a noobie here, and to programming in general but I've always heard that it was better to have a few sentences describing what is going to happen in the next few (especially complex) lines. I am certainly guilty of this kind of commenting, generally when refactoring others code. I think it's mostly because I want to remember how it's all working when I get it figured out. And secondly because most of the code I've looked at has always been commented so poorly (non-existant) or has been the kind of comments that repeat what is being done, like:
    'Declare a string
    Dim SomeVariable as String
    What about some commenting best-practices?


  • @Aristotle Pagaltzis said:

    A good codebase has

    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    <FONT face="Courier New" size=2>great documentation without comments?  i agree code should be written to be self-documenting.  however, in particular, i hate chasing down function calls and exploring that great big ol' call tree in the sky.  comments are really good for implementation details and explaining the non-trivial algorithm used in psuedo-code.</FONT>

    <FONT face="Courier New" size=2>part of the problem with comments is most people template them or they become this hassle to mantain or put into the code, which devolves in either using none at all or creating retarded comments.  the book "code complete" by mcconnel has some interesting insights into good commenting.</FONT>



  • @Aristotle Pagaltzis said:

    A good codebase has

    • very well-chosen identifiers
    • great documentation
    • very few comments, which only
      • explain why something happens or
      • warn of possible traps/bugs

    Tons of comments is a red flag in and of itself.



    I agree, but I think there are cases where "what" comment are also useful. An example from code I've just written:

    <font size="1">       < snip >
            /**
             * Creates and initializes a StateInfo for the specified class by
             * calling the various init-Methods.
             *
             * @param clazz
             *            a class that implements {@link StateSupport}
             *
             * @see #initParent()
             * @see #initStateInterface()
             * @see #initStates()
             * @see #initStateConstructors()
             * @see #initNullState()
             */
            public StateInfo(Class<? extends StateSupport> clazz) {
                statefulClass = clazz;
                // set the parent field
                initParent();
                // identify the annotated state interface
                initStateInterface();
                // look for declared states and their constructors
                initStates();
                initStateConstructors();
                // compute state visibility
                initVisibleStates();
                // create proxy for null state
                initNullState();
            }
           < snip >
            /**
             * Initializes {@link #visibleStateClasses}.
             *
             * @throws IllegalStateClassHierarchyError
             *             if assertions are enabled and a state that hides an
             *             inherited state by name does not extend the hidden state.
             */
            private void initVisibleStates()
                    throws IllegalStateClassHierarchyError {
                if (stateInterface != null) {
                    // this one is easy: all declared states are also visible, and
                    // only those
                    visibleStateClasses = stateClasses;
                } else {
                    if (stateClasses.length == 0) {
                        // no new state classes declared
                        // again easy: all visible states of the parent are visible
                        visibleStateClasses = parent.visibleStateClasses;
                    } else {
                        // new declared states will hide states of the parent, with
                        // the same simple name
                        visibleStateClasses = mergeDeclaredStatesToParentsVisibleStates();
                    }
                }
            }
           < snip >
    </font>  
    In the case of the constructor I first outlined the "algorithm" what needs to be done using comments, then I wrote the code, then refactored and put the parts into their own private final methods. I left the comments (almost unchanged), because it may help another developer to grasp the algorithm without looking into the initXXX methods. I don't think they hurt anybody if left there.

    In the case of the initVisibleStates method I felt the need to explain in a a few words, what the if conditions mean for the task to compute the correct visibilty.

    BTW: the whole .java file has >800 lines and contains about 30 methods, and presented here are two of three methods that have (non-javadoc) comments.


    OK, now laugh at my ignorance. ;-)

    cu


  • Well, those do look exactly like the sort of comments I’d try to avoid. If you wrote the comments first and the code afterwards, why are your identifiers less descriptive than your comments? Assuming, of course, that the methods you call don’t have documentation that would make it plainly obvious what’s happening anyway.

    At some point you’ll make changes to this code. You’ll either need to update or remove the comments. If you update them several times, each time after not having touched the code for a while, chances are the comments will say something else than what the code does. And there is no debugger for comments. That’s why “babysitter comments” aren’t just fluff, but actively harmful.

    When I inherit a big ball of mud, the first thing I do is remove all comments before I start reading. Only if I ever get stuck do I refer back to the codebase with comments, and then I only trust the comment as far as I can spit.

    Code is inherently hard to read. Comments lull readers into complacency. Learn to make code easier to read instead and use comments only when they really are absolutely necessary. That is much, much harder, but also much, much more beneficial.



  • This is a piece of code I got from a colleague. It was without comments. I added what I think is appropriate commenting. Any comments? [:P]

    <FONT color=#0000ff size=2>

    Private</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Sub</FONT><FONT size=2> btnOK_Click(</FONT><FONT color=#0000ff size=2>ByVal</FONT><FONT size=2> sender </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> System.Object, </FONT><FONT color=#0000ff size=2>ByVal</FONT><FONT size=2> e </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> System.EventArgs) </FONT><FONT color=#0000ff size=2>Handles</FONT><FONT size=2> btnOK.Click

    </FONT><FONT color=#0000ff size=2>Try
       </FONT><FONT color=#0000ff size=2>Dim</FONT><FONT size=2> strLogin </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>String

       </FONT><FONT color=#008000 size=2>' Login.Normal can take a while, so whow a message and a wait cursor
       </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.Cursor = Cursors.WaitCursor
       </FONT><FONT size=2>txtPassword.Visible = </FONT><FONT color=#0000ff size=2>False
       </FONT><FONT size=2>lblPassword.Text = objLng.LookUp("LOGON*VERIFY")
       Application.DoEvents()

    </FONT><FONT color=#008000 size=2>   ' Attempt to do a normal login (as opposed to an autologin)
       </FONT><FONT size=2>strLogin = Login.Normal("administrator", txtPassword.Text)

       </FONT><FONT color=#008000 size=2>' Login attempt complete, show normal UI again
       </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.Cursor = Cursors.Default
       txtPassword.Visible = </FONT><FONT color=#0000ff size=2>True
       </FONT><FONT size=2>lblPassword.Text = objLng.LookUp("LOGON*PASSWORD")
       txtPassword.Text = ""
       Application.DoEvents()

       </FONT><FONT color=#0000ff size=2>If</FONT><FONT size=2> Len(strLogin) = 0 </FONT><FONT color=#0000ff size=2>Then
          </FONT><FONT color=#008000 size=2>' Login.Normal returns Nothing or "" on success
          </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.DialogResult = DialogResult.OK
       </FONT><FONT color=#0000ff size=2>Else
          </FONT><FONT color=#008000 size=2>' Login.Normal returns the reason for a failure when it fails
          </FONT><FONT size=2>MsgBox(strLogin, MsgBoxStyle.OKOnly </FONT><FONT color=#0000ff size=2>Or</FONT><FONT size=2> MsgBoxStyle.Information, objLng.LookUp("LOGON*FAILED"))
       </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>If
       
       </FONT><FONT color=#008000 size=2>' After three failed logins quit the program
       </FONT><FONT color=#0000ff size=2>If</FONT><FONT size=2> Login.AmountOfLogins = 3 </FONT><FONT color=#0000ff size=2>Then
          </FONT><FONT color=#0000ff size=2>Me</FONT><FONT size=2>.DialogResult = DialogResult.Cancel
       </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>If
    </FONT><FONT color=#0000ff size=2>
    Catch</FONT><FONT size=2> ex </FONT><FONT color=#0000ff size=2>As</FONT><FONT size=2> Exception
       MyError.Show(ex.Message, </FONT><FONT color=#0000ff size=2>True</FONT><FONT size=2>)
    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Try

    </FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2>End</FONT><FONT size=2> </FONT><FONT color=#0000ff size=2>Sub</FONT>

    <FONT color=#0000ff size=2></FONT> 

    <FONT color=#0000ff size=2>Drak

    </FONT>


  • I think that's fine, and I disagree with most of what people have said in this thread.

    Code without comments fails my code reviews. Period. If I can't understand what your function does and how it does it after wiping out all the code and reading *only* the comments, then you did it wrong.

    Yes, you may have to update the comments when you maintain the code. So? You simply update the comments when you maintain the code.

    Now, I disagree with some of our practices in our code shop, such as the need for the psuedocode in the function header comment, but for the most part, code should be well commented because you're probably not the one whos going to be reading it in a year or two and trying to fix some weird obscure bug.

    Comments are invaluable in understanding how the code works on a high level and in helping somebody get down to the low level. I can't find a bug if I don't know where to look, and I won't know where to look if there's no comments to give me a general overview of how the thing works.

     



  • You make a strong point.



    I tried it on a javascript web-interface I wrote four months ago, and it looks like this:



       ///// Set up klanttypes /////



       // Type globals

       //easily limit selections to the right DIV

      

       //preget maximum id to allow SQL-style auto-incrementing after deleting items



       //collect id numbers to get largest.



       // Set click event for displayed list items (both lists)



       // Set click events for edit buttons (for both lists)



       // What to do when a button is clicked

       //actionRaw is a string of the format 'kt_update' or 'ln_add'

        // 'klanttypes' or 'talen'

       //add, update, delete

       //'kt' or 'ln'

       //get interface' editfield

       //IE keeps adding dead spaces to the end. Drat!



       // the action switch.

       // return; statements exit the function so that no data is submitted on error.



       //translate div ids into DB table names.



    Seeing it like this, there's a comment or two missing near the actual end, and it may not be as super-clear as possible.



  • When wiping out code to read comments, create some kind of macro that will replace the code itself with whitespace. The idea is to preserve the spacing in the function, which actually makes it much easier to see where the loops are as well as to tell where the "end of line" comments are that don't document the actual flow of the program.

    This helps me a lot when I'm trying to understand my own older code that I've forgotten everything about.

     



  • @Drak said:

    It was without comments. I added what I think is appropriate commenting. Any comments?

    The first and second comments are good. The third is maybe a bit verbose (given the first comment, it would suffice to say “reset UI”), and the rest of the comments below I find redundant.

    @Otto said:

    If I can't understand what your function does and how it does it after wiping out all the code and reading only the comments, then you did it wrong.

    If I need comments to understand the code, then you did it wrong, likely because you were trying way too hard.

    After all, how does reading the comments help if they say something completely (or subtly!) different from what the code does? The code is the only reliable source of information about what the code does.

    Comments are for decoration purposes, when something particularly tricky has to be done and can’t be written in any clearer fashion.

    Yes, you may have to update the comments when you maintain the code. So? You simply update the comments when you maintain the code.

    Except when you update the comments after a bugfix that comes a year after the last time someone touched the codebase, and then update the comments again another year down the road. Because chances are your updates to the comments will be lies and damned lies, and there’s no debugger for comments.

    Comments are invaluable in understanding how the code works on a high level and in helping somebody get down to the low level. I can't find a bug if I don't know where to look, and I won't know where to look if there's no comments to give me a general overview of how the thing works.

    That’s called documentation. Comments are entirely the wrong place for things like that.



  • If I need comments to understand the code, then you did it wrong, likely because you were trying way too hard.


    No.
    I understand and agree to the importance of self-documenting code, but inorder to understand code, one has to be focused. Often, you don't have time to be that focused, and you need a hand to quickly get to a suspect bit of code and start bugzapping, or quickly get to the code that needs finishing and start coding.

    After all, how does reading the comments help if they say something completely (or subtly!) different from what the code does?


    How can anyone in his/her right mind write a comment that does not summarize what the bit of code does?

    I mean something like this:
    [code]
    //destroy user object, close DB connections
    var philsSammich = new Sandwich(philsSpecialIngredients);
    philsSammich.addExtra(genericExtras.ketchup,extrasQuantity.britishtablespoon(6));
    deliveryList.push(philsSammich);
    [/code]

    That... doesn't happen.
    And if it does, you call it a 'bad comment'. The way you're stating it it's like no such thing as a 'good comment' exists.

    Comments are for decoration purposes, when something particularly tricky has to be done and can’t be written in any clearer fashion.


    You contradict yourself somewhat here. You say comments are decoration and then explain it by saying comments are for clarification. Of course tricky code has to be clarified, but when you're starting work on code, and you're scanning it as opposed to concentratedly reading it char by char, virtually [i]all[/i] code is 'tricky'. Especially around 4pm and you haven't had your cup-a-soup yet.

    That’s called documentation. Comments are entirely the wrong place for things like that.


    I'm not privy to standard procedure in software development because I'm just a maggot webcoder, but does the documentation contain an explanation of the bit of code, plus the line on which it sits? But that would be exactly what a comment is for. It's why it's called a 'comment'.


  • I agree with dhromed
    on comments. Providing a quick outline of a function can really help
    clear things up. When I'm starting to write a function, sometimes I'll
    write the comments first as steps, and then I'll fill those steps in
    with code. It can make for a very easy time coding, plus your docs stay
    in sync on your first write.



    If it were up to me, ALL documentation would be in the code. Tools like
    doxygen are so helpful for building your docs from the comments, and it
    seems like a hassle to do it any other way, as both code and
    documentation are right at your fingertips as you do your work.


    								</span>


  • @Otto said:

    Code without comments fails my code reviews. Period.
    If I can't understand what your function does and how it does it after
    wiping out all the code and reading only the comments, then you did
    it wrong.


    If the code is lucid then what purpose do the comments serve?  Comments should provide something the code doesn't.



    I use them for overviews and explanations of non-obvious sections, and
    for formatting.  Some code is obvious and needs no extra
    explanation to a competent programmer.  I'd say your policy is
    rather extreme.



  • @Aristotle Pagaltzis said:

    @Drak said:
    It was without comments. I added what I think is appropriate commenting. Any comments?

    The first and second comments are good. The third is maybe a bit verbose (given the first comment, it would suffice to say “reset UI”), and the rest of the comments below I find redundant.

    I thought personally that this comment was also useful:

    ' After three failed logins quit the program
       <FONT size=+0>If</FONT><FONT size=+0> Login.AmountOfLogins = 3 </FONT><FONT size=+0>Then
          </FONT><FONT size=+0>Me</FONT><FONT size=+0>.DialogResult = DialogResult.Cancel
       </FONT><FONT size=+0>End</FONT><FONT size=+0> </FONT><FONT size=+0>If
    </FONT><FONT size=+0>
    Returning a Cancel result does not always quit the program, but in this case it will... Ofcourse that's only true in this case, so maybe the comment is actually just WRONG?</FONT>

    Drak<FONT size=+0>

    </FONT>


  • @dhromed said:

    Often, you don’t have time to be that focused, and you need a hand to quickly get to a suspect bit of code and start bugzapping, or quickly get to the code that needs finishing and start coding.

    Personally, I prefer to have more than a superficial understanding of the code before I try to fix a bug…

    How can anyone in his/her right mind write a comment that does not summarize what the bit of code does?

    Because code has bugs. Why would comments be immune to them? There is plenty of room for discrepancies when the operation being both described and implemented is significantly more convoluted than your example. Also, since you propose fixing bugs without taking the time to understanding the code thoroughly, that opens yet more room for additions or changes to the comments to diverge from what the code really does.

    The way you’re stating it it’s like no such thing as a ‘good comment’ exists.

    The way I’m stating it is that all comments rot over time, regardless of how well they started out. The more revisions a codebase has been through, the more the comments become a liability.

    Of course tricky code has to be clarified, but when you’re starting work on code, and you’re scanning it as opposed to concentratedly reading it char by char, virtually all code is ‘tricky’. Especially around 4pm and you haven’t had your cup-a-soup yet.

    Then you shouldn’t be coding at the time. You are supposed to make a change, not add bugs.

    Note that I’m not saying that comments are bad. I’m just saying they’re very easy to abuse. I too used to think that commenting profusely is a good idea because there's no harm in having a lot, but there would be in having too little. But there is harm; comments are like traffic signs. Too many and they’ll whirr by in a blurr, signifying nothing; although city planners might be tempted to use them as a crutch for made mistakes.



  • I don't really agree that code is hard to read.

    Well, of course it depends on the language, Assembler is really hard to read, but I mostly code in Python and unless some special magic happens I find that it isn't really hard.

    Of course you can't read it like written text, you'll have to think a bit. But comments like:

    ### If actor is attacking, handle the attack.
    if actor.state == "Attacking":
    
    ### Make the actor rotate
    actor.rotate(fTime)
    			
    ### Make the actor move
    actor.move(fTime)
    
    ### First we check if the actor has just been created
    ### Because that means we want to scale it to its default size
    ### from really small.. to create a cooler effect. :)
    if actor.state == "Being Spawned":
    	
    
    ### Then we check if the actor is dying
    elif actor.state == "Dying":
    	
    ### Then we check if the actor is landing
    elif actor.state == "Landing":
    	### Make the actor move
    	actor.move(fTime)
    	
    

    really doesn't help that much to me. I want comments that tell what the code does not, otherwisw I'll just have to read each commend twice, once in comment form and once in code form to make sure that the comment was accurate and that nothing more happen than mentioned.

    Personally I have a tendancy to comment way to little though, well, actuall on my last project I realised that I hadn't commented at all, that probably was a bit too little, even though I work hard on making sensible code that doesn't need comments.



  • Tally another point for the "comments for almost every line" crowd, although with a big caveat to this.

    I like to code as english as possible, so my inital methods read as pure english so no comments are needed.  These methods are composed of 3 or 4 words.  I'd also like to point out this is a new way of coding for me.

    These methods also read very english like and somebody call a 3rd tier of methods.  This 3rd tier is the actualy code which is commented on about 80% of the lines.  Just by reading the comments only and skipping the code, you can understand exactly what my code does.

    I can't really explain why but I really love to have my initial loading method read as such straigt forward english, that even a ProjectManager/CIO could read the code and know exactly what it does.  A more technical person could dive down into the 2nd layer of methods and get a more step by step overview.  And a true coder would dive into the 3rd layer and see the actual line by line code.

    Also on a dev tangent, I've been trying to write my code from the top down.  So instead of writing lines of functional code, I write the overly english methods and create empty placeholders (sometimes containing hard coded example data so I have a functioning demo).  I find this to be more fun and productive than my old way of coding which was along the lines of: my code needs to do 1, 2 and 3. Code all of 1 first, then all of 2, then 3.

    I've come to dislike writing my apps 1 line at a time as I feel keeping a broad overview of the app and as I develop more of the app, I get deeper into the code.

    If you've never coded this way, give it a try, you might like it better like myself.



  • @Jonex said:

    I don’t really agree that code is hard to read.

    How large is the most complex program you’ve ever written?

    A piece of code about 20× as long as your example that actually does something is about the minimum size of a unit of code you need to understand all at once in a typical program that does more than one simple thing. (And I don’t mean in Java – I mean in dense languages like Perl, Python, Ruby, or the likes.)

    That is certainly not going to be so trivial to understand as your contrived example. Note that I didn’t say code is impossible to read; I said code is hard to read. If something involved is going on, regardless of how simply you write it and whether you use an inherently expressive language, the code will always require concentration and careful reading to understand.

    Not that I know what relation to the topic the point you’re making has – if code was easy to read, we wouldn’t be having this discussion about commenting.



  • @Aristotle Pagaltzis said:

    @Jonex said:
    I don’t really agree that code is hard to read.

    How large is the most complex program you’ve ever written?

    A piece of code about 20× as long as your example that actually does something is about the minimum size of a unit of code you need to understand all at once in a typical program that does more than one simple thing.  (And I don’t mean in Java – I mean in dense languages like Perl, Python, Ruby, or the likes.)


    Yikes.  Just because you can write very dense code with highly expressive languages doesn't mean you should.

    @Aristotle Pagaltzis said:

    That is certainly not going to be so trivial to understand as your contrived example. Note that I didn’t say code is impossible to read; I said code is hard to read.  If something involved is going on, regardless of how simply you write it and whether you use an inherently expressive language, the code will always require concentration and careful reading to understand.

    I don't think your conclusion follows from your argument.  I think it's more likely that certain algorithms require concentration and careful reading to understand, and that it is the algorithm -- not the code -- that is hard to understand. To assert that all code is hard to read because some algorithms are involved is certainly an error.  To assert that all code is involved and complex is also an error.

    I've seen extremely simple systems blown into complex maintenance nightmares by programmers that had a poor understanding of the fundamentals of abstraction.  When you get down to it, some business processes are indeed complex and certainly require much careful reading and concentration.  But code for these algorithms should still be easy to read.

    @Aristotle Pagaltzis said:

    Not that I know what relation to the topic the point you’re making has – if code was easy to read, we wouldn’t be having this discussion about commenting.

    Nonsense.  Some code is easy to read and some code is difficult to read.  Not all code is created equal.



  • @Aristotle Pagaltzis said:

    @Jonex said:
    I don’t really agree that code is hard to read.

    How large is the most complex program you’ve ever written?

    A piece of code about 20× as long as your example that actually does something is about the minimum size of a unit of code you need to understand all at once in a typical program that does more than one simple thing. (And I don’t mean in Java – I mean in dense languages like Perl, Python, Ruby, or the likes.)

     

    Dude. Do you code in Python? I can write the SMTP/IMAP handling section of an email client in 30 lines in Python, 5 of which are comments. My GUI runs to about 50 lines, including comments.

    Do you really need me to comment -

    imapObj.login(user, pwd)

    to tell you that imapObj is an IMAP connection object, and you're logging in using a username and password? I find that kind of commenting either insulting or annoying.

    Whereas, I will comment something like this -

    data = imapObj.fetch(msg, 'RFC822.TEXT')[1][0][1] 

    Because it's not immediately obvious that an imap object returns a tuple of

    (status code, ((some junk, actual data), ')' )

    (And no, I have no idea why it returns like that.
    I've been meaning to subclass imaplib.IMAP4 for awhile now.)

    I'll also comment what the hell RFC822.TEXT is as well.

    Comments should clarify. If you can't understand what a function does without every single line being commented, why are you reading code?



  • @Aristotle Pagaltzis said:

    How large is the most complex program you’ve ever written?

    Not very large. When I have a complex problem I'll usually divide it into smaller, less complex pieces. That way I make the actual [i]source[/i] easier to read, instead of solving the problem in a complex and making the comments do the abstraction.

    @Aristotle Pagaltzis said:
    That is certainly not going to be so trivial to understand as your contrived example.

    Contrived? That was from actual source code from a project I was looking at.

    Note that I didn’t say code is impossible to read; I said code is hard to read. If something involved is going on, regardless of how simply you write it and whether you use an inherently expressive language, the code will always require concentration and careful reading to understand.

    @Aristotle Pagaltzis said:
    Not that I know what relation to the topic the point you’re making has – if code was easy to read, we wouldn’t be having this discussion about commenting.

    Obviusly not all code is easy to read. But my point is that [i]good[/i] code should also be easy to read, otherwise it isn't good code.(sometimes you have to make exceptions though, of performace reasons for instance) No matter how much comments you put in it.

    A good general rule is that when you feel the need of adding comments, check if you can make the code clearer by changing variable names or refactoring or in other ways change your code instead. That will not only make the code readable but probably more maintanable as well.

    Note, though, that I'm coming from a Python programming perspective. Python's syntax is made with readability in mind, moreso than many other languages. So I'm sure there can be a larger need for comments in less readable languages. (Assembler as the most obvius example.)



  • Okay, several people argue “against” me with my own arguments and I need to quote long pieces of text to maintain context, so this thread seems to have about run its course… anyway, here goes:

    @dhromed said:

    How can anyone in his/her right mind write a comment that does not summarize what the bit of code does?

    Well, I’m coming back to this. Someone (dhromed or someone who agrees with him) explain to me how this doozy from Kasia’s weblog came about:

    //select query formed. MEADOW _ID 2067 NEED TO CHANGE IT IF IT DOES NOT EXIST IN DATABASE.
    selectQuery = "SELECT LOCATION FROM MEADOWS WHERE MEADOW_ID=" + meadow_id;
    //Update Query formed .MEADOW_ID 2067 NEED TO CHANGE IT IF IT DOES NOT EXIST IN DATABASE.
    updateQuery = "";
    

    The second comment is obviously lying. How did the programmer manage to do that? Don’t ask me, but clearly, these comments that having nothing to do with what the code next to them is doing do exist, in production code no less.

    @Chris F said:

    @Aristotle Pagaltzis said:
    If something involved is going on, regardless of how simply you write it and whether you use an inherently expressive language, the code will always require concentration and careful reading to understand.

    I don’t think your conclusion follows from your argument.  I think it’s more likely that certain algorithms require concentration and careful reading to understand, and that it is the algorithm – not the code – that is hard to understand. To assert that all code is hard to read because some algorithms are involved is certainly an error. To assert that all code is involved and complex is also an error.

    Point granted – it’s mainly the algorithm that’s hard to read.

    But I don’t think that really negates what I’m saying. What I’m asserting is that expressing a complex algorithm in code inherently introduces a layer of concreteness that shrouds the intent of the code. I’m not sure I can explain this adequately. By way of an example, take an algorithm such as “go from home to the bus station” – for a computer, you have to express this as a series of concrete instructions, so you end up with something like “starting at the location home, while your location is not the bus station, determine all adjacent locations, select the one which is closer to the bus station, move there, and repeat.” The reader has to deduce the abstract intent from these concrete instructions. That’s what goes on when you read code: you need to deduce, by tracing execution from one concrete instruction to the next, what the code is really intended to do. That’s why code is hard to read – it conceals, by the very virtue of being instruction code, what its purpose is. More expressive languages help (I find well-written Perl very natural to read), but they too cannot excise this.

    @Cyresse said:

    @Aristotle Pagaltzis said:

    A piece of code about 20× as long as your example that actually does something is about the minimum size of a unit of code you need to understand all at once in a typical program that does more than one simple thing. (And I don’t mean in Java – I mean in dense languages like Perl, Python, Ruby, or the likes.)

    Dude. Do you code in Python? I can write the SMTP/IMAP handling section of an email client in 30 lines in Python, 5 of which are comments. My GUI runs to about 50 lines, including comments.

    Okay, so the example if referred to was 8 LoC, and your mailer is a total of 80 lines, so my estimate was off by a factor of 2. Not all that bad, if you ask me, and the exact factor would depend on the kind of application.

    Do you really need me to comment -

    imapObj.login(user, pwd)

    to tell you that imapObj is an IMAP connection object, and you’re logging in using a username and password? I find that kind of commenting either insulting or annoying.

    Whereas, I will comment something like this -

    data = imapObj.fetch(msg, 'RFC822.TEXT')[1][0][1] 

    Because it’s not immediately obvious that an imap object returns a tuple of

    (status code, ((some junk, actual data), ')' )

    (And no, I have no idea why it returns like that.
    I’ve been meaning to subclass imaplib.IMAP4 for awhile now.)

    I’ll also comment what the hell RFC822.TEXT is as well.

    Comments should clarify. If you can’t understand what a function does without every single line being commented, why are you reading code?

    Uhm, did you read the thread? Those are my points exactly. :-) I’m the one who has spent all this time arguing that excessive commenting is harmful and that commenting in general should be done with extreme care.

    For the record, data structures should be documented thoroughly. Thanks for reminding me of this subtlety: good code means investing a lot of effort in good data structures, so that the code has little work to do. If the data structures are exhaustively documented (not commented), the code will need relatively little in the way of clarification. C.f. Fred Brooks.

    @Jonex said:

    @Aristotle Pagaltzis said:
    How large is the most complex program you’ve ever written?

    Not very large. When I have a complex problem I’ll usually divide it into smaller, less complex pieces. That way I make the actual source easier to read, instead of solving the problem in a complex and making the comments do the abstraction.

    Wrong argument, really. Even with a broken-down codebase, you need to keep enough in your head at once to understand the interactions. And my assertion that a non-trivial unit of code comes to about 150-200 LoC even in a dense language is, I think, hard to deny.

    Obviusly not all code is easy to read. But my point is that good code should also be easy to read, otherwise it isn’t good code. (sometimes you have to make exceptions though, of performace reasons for instance) No matter how much comments you put in it.

    I think we have a semantic confusion here. When I say “easy to read” I really mean “easy to understand”. With that in mind I defer to my response to Chris F, above, about this point.

    Anyway, this is really all moot, because…

    A good general rule is that when you feel the need of adding comments, check if you can make the code clearer by changing variable names or refactoring or in other ways change your code instead. That will not only make the code readable but probably more maintanable as well.

    Well, like Cyresse, if you actually read the thread through from top to bottom, you would find that I have been arguing precisely this all the time.

    Note, though, that I’m coming from a Python programming perspective. Python’s syntax is made with readability in mind, moreso than many other languages. So I’m sure there can be a larger need for comments in less readable languages. (Assembler as the most obvius example.)

    I’m coming at this from the Perl programmer perspective.

    Yes, really. :-) The same syntactic freedom that is frequently abused for clever-looking or obfuscated code can also be used as an effective tool to improve the clarity of code. It just takes discipline. (And I resent tools that presume that I lack discipline; hence Perl, and not Python. If you really want to torture me, make me write Java. But Perl is not without flaws, I just like it better, warts and all. To each his own.)



  • @Aristotle Pagaltzis said:

    Okay, several people argue “against” me with my own arguments and I need to quote long pieces of text to maintain context, so this thread seems to have about run its course… anyway, here goes:

     

    I'd just like to say - mea culpa. Went back and read closer. This is what happens when I reply at 4 in the morning. I embarress myself.

     



  • Ack.... embarrass.



  • @Aristotle Pagaltzis said:


    But I don’t think that really negates what I’m saying. What I’m asserting is that expressing a complex algorithm in code inherently introduces a layer of concreteness that shrouds the intent of the code. I’m not sure I can explain this adequately. By way of an example, take an algorithm such as “go from home to the bus station” – for a computer, you have to express this as a series of concrete instructions, so you end up with something like “starting at the location home, while your location is not the bus station, determine all adjacent locations, select the one which is closer to the bus station, move there, and repeat.”

    Or you write a function that does that goFromHomeToBusStation() you'd probably make it more abstracted though, something like. somePerson.go(places.busStation) (assuming where he goes from isn't really of importance)

    The function may be a bit less readable, but it's intent should be more or less described in the function name with any complex part within it refactored in the same way. Now I see that you'd want to complement with docstrings for those cases where the intent and result of the function isn't obvius, but I don't think that's the same as having inline comments in the middle of a function. Code can be just as terse as the description of it is, if it isn't, make it so.

    But maybe I have misunderstood your opinion and we actually agree. :)

    @Aristotle Pagaltzis said:
    Wrong argument, really. Even with a broken-down codebase, you need to keep enough in your head at once to understand the interactions. And my assertion that a non-trivial unit of code comes to about 150-200 LoC even in a dense language is, I think, hard to deny.

    But my point is that all the info you can concentrate in comments, you can concentrate in code.

    # Do thing one
    someStatement1
    someStatement2
    someStatement3
    someStatement4
    ...
    # Do thing two
    someStatement1

    someStatement2

    someStatement3

    someStatement4
    ...
    Could also be written as:
    def ThingOne(*args):
       someStatement1
       someStatement2
       someStatement3
       someStatement4
    def ThingTwo(*args):

       someStatement1

       someStatement2

       someStatement3

       someStatement4

    ThingOne(args)
    ThingTwo(args)
    @Aristotle Pagaltzis said:
    Well, like Cyresse, if you actually read the thread through from top to bottom, you would find that I have been arguing precisely this all the time.

    I don't get it then, do you think code is inheritely hard to read or, as I do, that only [i]bad[/i] code is inheritely hard to read and that it's practically always possible to make it easy to read with some effort. Except when you are locked into a language that won't lend itself to make the code easily readable.

    I'm not really arguing against you, I'm just arguing agaist what you say. When you say code is hard to read, I disagree on that point, even if we agree on the usage of comments in real code.

    @Aristotle Pagaltzis said:


    I’m coming at this from the Perl programmer perspective.

    Yes, really. :-) The same syntactic freedom that is frequently abused for clever-looking or obfuscated code can also be used as an effective tool to improve the clarity of code. It just takes discipline. (And I resent tools that presume that I lack discipline; hence Perl, and not Python. If you really want to torture me, make me write Java. But Perl is not without flaws, I just like it better, warts and all. To each his own.)


    It isn't the required discipline that makes me dislike languages like Perl. It's the fact that I always get lots of choices. I'm good at logic and very ba at choices, so every time I get a hard choice I lose concentration and coding speed. Therefore I really don't like useless choices, choices that really doesn't matter and that can not easliy be solved by logic. That's probably why I dislike comments too, I don't know a logic method of knowing where to use them, doc-strings are much simpler in that manner.
    But as you said, to each his own. Python got it's flaws too, I'm not very fond of the __'s usage nor the inconsisteny between mutable and unmutable types.



  • @Jonex said:

    I'm not very fond of the __'s usage nor the inconsisteny between mutable and unmutable types.

    Ehyah, I think 90% of the trouble of trying to understand what's happening in a metaclass is all the __'s in the code.


    But out of regard to the mutable stuff, other than strings and tuples what else is immutable?



  • I realise that the above example is a little ridiculous but does it
    really matter as it wont really make much difference to your program as
    comments are usually ignored by the compile.. 




  • @Jonex said:

    ...that only [i]bad[/i] code is inheritely hard to read and that it's practically always possible to make it easy to read with some effort.

    How about this, then?  The following method is called three times during creation of a graphic 90-day calendar (it's called once for each of the three months displayed).  Here's what the calendar looks like.  Note that September's display of days overlaps into August but not into October.  This is because August is not present in the display, but October is.  October doesn't overlap in either direction, and November displays days from December because December is not present in the display.  The properties ShowPreviousMonth and ShowNextMonth, and other relevant values, have already been set for the month being processed before the method is called.

    Now, imagine you wrote this code two years ago, and haven't seen it since, and now you're coming back to it.  How understandable is it?

    <FONT face="Courier New" size=2>public void getEvents()
    {
     int day;
     int month;
     int year;</FONT>

    <FONT face="Courier New" size=2> if ( this.ShowPreviousMonth && this.FirstDayIndexIntoWeek != 0 )
     {
      day = ( (30 + this.extraDaysInMonth[this.LastMonth]) - 7 );
      month = this.LastMonth;
      year = this.YearForLastMonth;
     }
     else
     {
      day = 1;
      month = this.Month;
      year = this.Year;
     }
     DateTime startDate = DateTime.Parse( month.ToString() + "/" + day.ToString() + "/" + year.ToString() );</FONT>

    <FONT face="Courier New" size=2> if ( (this.ShowNextMonth) && ( (this.FirstDayIndexIntoWeek + this.DaysInMonth) % 7 != 0 ) )
     {
      day = 7;
      month = this.NextMonth;
      year = this.YearForNextMonth;
     }
     else
     {
      day = this.DaysInMonth;
      month = this.Month;
      year = this.Year;
     }
     DateTime endDate = DateTime.Parse( month.ToString() + "/" + day.ToString() + "/" + year.ToString() );</FONT>

    <FONT face="Courier New" size=2> _data.getEvents(this.ModuleId, FacilityIDs, startDate, endDate);
    }</FONT>

    I can't speak for you, but I'd be reading through it asking, "WTF was I doing here?"  It will help me immensely in dusting away the mental cobwebs if I have the following comments in place (which I do in the actual source):

    <FONT face="Courier New" size=2>/// <summary>
    /// Obtains an EventCollection for the current month and possibly overlapping
    /// a week into the previous or next months.  Overlap is determined by the
    /// value of the properties ShowPreviousMonth or ShowNextMonth as well as
    /// FirstDayIndexIntoWeek (the position in the week of the first day; if it
    /// occurs on Sunday there will be no need to show the previous month's days)
    /// and also the position in the week of the last day of the month.  Events are
    /// filtered based on the FacilityIDs collection.
    /// </summary>
    public void getEvents()
    {
     **** snip ****</FONT>

    <FONT face="Courier New" size=2> // Determine whether we need to retrieve events from the previous month.
     // Even if ShowPreviousMonth is true, there is no need to retrieve the
     // previous month's events if this month's first day is a Sunday (index 0).
     if ( this.ShowPreviousMonth && this.FirstDayIndexIntoWeek != 0 )</FONT>

    <FONT face="Courier New" size=2> **** snip ****</FONT>

    <FONT face="Courier New" size=2> // Determine whether we need to retrieve events from the following month.
     // Even if ShowNextMonth is true, there is no need to retrieve the
     // next month's events if this month's last day is a Saturday (index 6),
     // determined by adding DaysInMonth and the offset of the first day into
     // the week, dividing the result by 7.  A result of 35 will return a modulus
     // of 0, indicating the last day is at index 6).
     if ( (this.ShowNextMonth) && ( (this.FirstDayIndexIntoWeek + this.DaysInMonth) % 7 != 0 ) )</FONT>

    <FONT face="Courier New" size=2> **** snip ****
    }
    </FONT>



  • I wouldn't use the bare code, it definitely needs comments. But, if
    what i'm pulling from your commented example is correct, you have
    around 6 lines of comments for every if statement. That's absurd. 
    The <summary> is good, but I personally would do something like

    // Do we need to retrieve events from the previous month?

    instead of the 3 lines, and probably cut the example bit out out of the
    admittadly non-intuitive modulo. But, to each his own. More important
    then comment debate, what matters is whether your style helps you, or
    gets in your way. If I don't like your comments, I can just grep them
    out. :)



  • @Spigot said:

    But, if what i'm pulling from your commented example is correct, you have around 6 lines of comments for every if statement.

    Actually, the comments pertain to an entire block of code, not just one line.  Comments irritate me because they interfere visually with the linear flow of code, so I use them sparingly.  When I do comment, I try to make it a complete enough explanation that it will answer my questions when I return to the code later, having forgotten the logic flow.

    @Spigot said:

    More important then comment debate, what matters is whether your style helps you, or gets in your way. If I don't like your comments, I can just grep them out. :)

    Truly spoken.  I responded because I disagree that only bad code needs commenting.  I believe I've given an example to the contrary.


Log in to reply