The Worst Comments

  • 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

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Gary Varga (3/13/2015)


    I was going to try and say "no comment" in a witty way but as I am not first to highlight the issue (and I am not always that witty) I shan't bother. Instead I have the following anecdote:

    I have recently worked with people who have analysed code because of the lack of documentation (i.e. no specification and limited/poor/non-existent comments) and made paper notes then augmented the code with their own uncommented code. Muppets.

    I've run into that many times, as well. I love it when someone spends two days analyzing some nasty, large, totally uncommented code and it turns out that same way when they're done. I just don't understand it.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • The only thing wrong w/ bad comments is no comments at all.

    On a number of occasions I have found that various co-workers will add a comment that they made various enhancements, but not reference where the various changes were made. Arrgggghhh!

  • Michael.Ramirez (3/13/2015)


    The only thing wrong w/ bad comments is no comments at all.

    On a number of occasions I have found that various co-workers will add a comment that they made various enhancements, but not reference where the various changes were made. Arrgggghhh!

    I disagree. Reading bad comments wastes time without imparting useful information.

    If you haven't even tried to resolve your issue, please don't expect the hard-working volunteers here to waste their time providing links to answers which you could easily have found yourself.

  • Bug 445: as per the operations group

    This one may or may not be a bad comment. It appears to be a reference to a tracking ticket. And rather than type twenty lines in the code, the author references the ticket so it can be looked up in the future.

    Makes me think that wiki style comments with inline links to tracking tickets or reference articles might not be a bad idea.

  • I found this one trying to debug some code...I loved it so much that I left it in

    SELECT @DEFAULT_DMR =....

    --ff**king useless suppliers

    DELETE FROM...

    without the asterisks of course

    😀

  • The worst comments are Old code (REAALLY OLD) from at least one system platform prior. I have dealt with crossover systems for most of my professional career, and I understand that software code evolves over the life time of the system, and sometimes a system gets upgraded to a new platform. and There is not time to deal with all the legacy parts of the code. But we had one system whose core was developed on a mainframe, then transitioned to VB3, and by the time I was working on it, The code platform was working on a VB6 w/ SQL Server and ADODO platform. There were comments in the system that were written in the original MUMPS, referring to database access through DEC Control Language (DCL). All the developers working on it, except my boss and I had never used MUMPS or DCL, so the database access comments were foreign to them; nevermind that we were accessing SQL 2000 and 2005 at the time. My grief was that the comments were never reduced to an abstraction to port across the platforms or deleted. Imagine finding something like this in your stored proc. (addresses and ports have been changed to protect the innocent and the guilty)

    -- S CONST="555.555.555.555:999999 1002,4,8,1",P="ETHER",S="19200" ;;JJ-bind

    -- S L="ASCII" ;;2/92-PW-link charset

    -- W:DBP CONST(P,L) ;;JJ-conn

  • I typically do not mind general comments as if someone pseudo coded a general approach and then wrote their code (at least they thought about it first).

    However, the comments I do not like are

    --TODO (with no notes on what was left to do or why)

    --Not sure why this code is here so I am leaving it (we should figure it out)

    Or general comments on how something is supposed to work that in no way matches what it actually does. Now you are left guessing if the process or rules changed and the comment was not updated or is the code just plain wrong, until you do a vast amount of research.

    I actually think comments at the top of code, or as check-in labels, that refer to help system tickets can be helpful since those tickets usually contain behavior or business process information as to why the change was made.

  • The absolute worst type of comment is when a developer encloses a pre-existing block of code within /* */ brackets without an additional comment stating why, when, and who.

    Another type of useless comment is the one that simply states the obvious. For example:

    -- Deleting from RunLog table where StartDate is older than 3 months.

    OK, I can see that, but when was this change made and why?

    With just a little more effort, here is a much better comment, and inclusion of the actual change order # provides a convenient link for research purposes, if needed.

    -- 2014/02/13 J.S. TFS34621: Performance optimization.

    -- Operations requested that history older than 3 months be purged from run log table.

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

  • Gary Varga (3/13/2015)


    I was going to try and say "no comment" in a witty way but as I am not first to highlight the issue (and I am not always that witty) I shan't bother.

    "Comments? We don't need no stinkin' comments."

    But I'm one that lives in a glass house so I shall throw no stones on this topic. :Whistling:

    -------------------------------------------------------------
    we travel not to escape life but for life not to escape us
    Don't fear failure, fear regret.

  • 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.

    -------------------------------------------------------------
    we travel not to escape life but for life not to escape us
    Don't fear failure, fear regret.

  • -- =============================================

    -- Author:<Author,,Name>

    -- Create date: <Create Date,,>

    -- Description:<Description,,>

    -- =============================================

    -- Add the parameters for the stored procedure here

    -- SET NOCOUNT ON added to prevent extra result sets from

    -- interfering with SELECT statements.

    (bonus points if SET NOCOUNT ON has been removed)

    -- Insert statements for procedure here

  • I used to keep a running list of the Top 10 Developer Comments that were in our codebase. It was shared with every developer that worked on our system. One of my favorites:

    <!---

    This is a good example of the stupid stuff we are forced to do here. Operations made

    me change this code from A to B to C to B to D and all the way back to A. I wasted

    weeks of work to end up with the same thing I started with. This is why I don't get

    rid of old code.

    --->

    And one that I had to leave in a few places:

    <!---

    Please don't blame me for the following code. XXXXXXX made me do it this way.

    --->

  • Somewhat related I guess as there was something to do but it was never done:

    // Don't touch: I don't know why this works.

    Followed by an obscure api call/bit twiddling (can't remember exactly). When the original developer doesn't know what it was that they were doing you are pretty lost.

  • I've seen that one too. I've dug into code to find a file that was nested several layers deep, only to find that the entire contents of that nested file had been commented out. :crazy:

Viewing 15 posts - 31 through 45 (of 156 total)

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