The Worst Comments

  • I don't know after the file has been edited another dozen times do you really want to walk through the source control history and try to figure out which code was trying to do the same thing before? Painful. Don't comment out code that never worked I agree. Often though code is around because of business rules that changed (and might not be well understood in the first place). Leaving the old code around in a visible place makes it obvious to the coder (who after all is only looking at the current version) what used to be the rules. I'd love for there to be a single location to find all business logic decisions and their history over time but I've never seen a place that works that way. Often the decision is a decision as you are walking down the hall. I'm not a secretary so the code has to be my documentation.

  • Graphic sexually explicit comments. Lots of them.

  • The real question is should you read and depend comments in code. Even a good comment could be left from previous versions and even small changes could change the functionalty and make the comments meaningless.

  • Ooh, here's some from a random proc I just plucked out of Object Explorer (stuff in "<>" is what I removed before posting). Mix of bad code/bad comments; I've stuck to the comments but must unavoidably include some coding gems.

    ---- Insert Addon of ProductBundle into table <removed>

    ---- using currsor to insert

    Then 15 variable declarations followed by:

    --- Currsor

    DECLARE @getAddonItem CURSOR

    SET @getAddonItem = CURSOR FOR ...

    Then just before the BEGIN TRAN that each pass through the cursor does, there's:

    --- select * from <arbitrary table>

    --- select * from <different arbitrary table>

    --Addon.Units

    Inside the transaction:

    --- Insert Addon

    INSERT INTO ...

    --- calcuation addon price for product bundle

    SELECT @Amount=<'100-line scalar function that takes 11 parameters, and coincidentally contains all the comments that indicate that it started life as a right-click in SSMS'>

    Finally:

    BEGIN CATCH

    ROLLBACK TRAN

    --PRINT ERROR_MESSAGE()

    RAISERROR('Error creating <object>', 10, 1)

    END CATCH

    COMMIT TRAN

    Needless to say that this procedure could be refactored into a single statement. It may come as a surprise to learn that some of our work is outsourced.

  • "Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho

  • Something I'm guilty of is a comment like this:

    -- If you get to this point, you have problems

    Beer's Law: Absolutum obsoletum
    "if it works it's out-of-date"

  • One of my favorites:

    /* Oh cruel Fortuna, if but I had the time to

    remove the foul dynamic sql from this travesty

    of a procedure. Alas, time is a scarcer

    commodity than humility in a 200 level poetry

    course.

    */

    I did take the time to rewrite the stored procedure. 🙂

  • below86 (3/13/2015)


    Phil Parkin (3/13/2015)


    Gary Varga (3/13/2015)


    Yes!!! Commented out code. That's what source code control is for. If you need to leave a reference to old code then just leave a comment WHY it was removed with a date. Commented out code alone just says "this is code we are not using". I don't care unless I know WHY and even then I don't want a third of the code to be that which is deemed no longer relevant!!!

    I think commented-out code says this:

    "I may have removed code which is still required – I'm just not sure. But if I leave it here, I lessen the problem, because it will be very easy for some other developer, who actually knows what they are doing, to resurrect. I don't really understand source control."

    😀

    Quilty as charged, often done since we don't have versioning software, you know, keep the code in case we ever need it again.

    Also guilty, but we do use source control. :Whistling: My typical process is to comment out code that I'm refactoring with a note about why it was refactored or where it was moved to and I'll leave it commented for one check-in. The next time I modify that section of code I'll delete any code that was commented. If I need to refer to it after that point, I'll search through source control to find it.



    The opinions expressed herein are strictly personal and do not necessarily reflect the views or policies of my employer.

  • Something that went along the lines of 'That's it, I've had enough, no one listens to me or answers emails and they all just moan. They can go back to the way it was before when they got bad data'

  • GilaMonster (3/13/2015)


    Toby Harman (3/13/2015)


    Many years ago I found a comment by my boss in the code which read something along the lines of

    -- Don't touch this code

    -- It does fancy statistical maths on an iterative loop until it gets the answer

    -- Really, don't touch it

    -- Even if you think you kn ow what you are doing, DON'T

    It served one useful purpose. It warned me that the next page was complex. It didn't tell me anything about what it was actually doing!

    I've done one of those some time back. It was only a ~8 line long SQL statement, but it had taken me a couple days to get it right. The comment just read

    -- Please don't change this unless you understand it completely

    I wrote some dBase 3 code for printing tax forms on an Okidata dot matrix printer. The code contains printer codes for controlling line spacing and such, it was the only way to get the information to register correctly on the form, it took a couple of weeks to get it right. And it contained massive warnings in the code to not touch the format code in this section or you're probably going to break it.

    After I left the company, the IT direct touched the code. He broke it. It never worked again. But they never called me to see if I could fix it, so I guess they went back to hand-writing 1099s. Broke my little heart. 🙂

    -----
    [font="Arial"]Knowledge is of two kinds. We know a subject ourselves or we know where we can find information upon it. --Samuel Johnson[/font]

  • Bad comments:

    - Comments that are factually incorrect

    - Commented code (anything that has actually been checked in with commented code)

    - Comments that are misspelled or use improper grammar (personal pet peeve)

    - Comments that restate the obvious

    - Comments including references to now-unused external systems (defect tracking, change requests)

    - Comments with the names of programmers (excluding headers and change logs that require a name)

    - Obscene comments

    - "I funny" comments (for example, "deez" instead of "these")

    How to get proper comments:

    - Keep boiler-plate comment requirements to a minimum (preferably none)

    - Choose self-documenting names for objects, methods and variables

    - Only write comments where the code does not self-document

    - Use a comment spell checker in the IDE (several are available for Visual Studio)

    - All code is reviewed before check-in

    - The code reviewer is fluent in the comment language (English, for example)

    - The code reviewer is willing to hold up the review until lousy comments are fixed

    - Management does not undermine the above

    And as for "no comments," I'm not sure that's always a bad thing. If the code is clean and names are well-chosen, then comments should be rare beyond whatever is required by the organization's coding standards. If the code is a mess, how are comments going to help? They can't be trusted. I've never seen messy code that had useful, valid and well-organized comments. If existing production code is a mess, then the only valid documentation of that system is the code itself. Period. Nobody knows what it really does, only what they think it does.

  • The most annoying was back in my RPG coding days. For those two young to remember, IBM RPG was a positional language originally designed for punch cards. With the terminal emulators we used, you could set an invisible bit prior to the spec column to change the color of that line of code. One of our contract developers was quite fond of this and often set lines to BLINK. There are few things more annoying than blinking comment code.

  • The file is empty. I know, I read it.

  • Tony Lanterman (3/13/2015)


    One of our contract developers was quite fond of this and often set lines to BLINK. There are few things more annoying than blinking comment code.

    Oh my. That's a 'take them out and shoot them' level offense. They can't be allowed to pollute the gene pool.:-P

    Sigerson

    "No pressure, no diamonds." - Thomas Carlyle

  • william.trudell 38765 (3/13/2015)


    The real question is should you read and depend comments in code. Even a good comment could be left from previous versions and even small changes could change the functionalty and make the comments meaningless.

    Code maintenance fail.

    Gaz

    -- Stop your grinnin' and drop your linen...they're everywhere!!!

Viewing 15 posts - 46 through 60 (of 156 total)

You must be logged in to reply to this topic. Login to reply