Lazycoder

1Jun/059

ending bracket comments

Am I the only person that does this?


                            }//end verifyCOREUser
                        }//end UrlReferrer check
                    }//end Querystring[pid]
                }//end IsAuthenticated
            }//end is HttpApplication

Man, I get lost in all those brackets if I don’t put stuff like that in.
It kind of clutters up the code, but it makes it easier to tell where you are in the bracket maze.

Filed under: General Leave a comment
  • http://www.codebetter.com/blogs/jay.kimble Jay Kimble

    Actually being a former VB guy I do this
    } // End While
    } // End If
    } // End Function

  • http://www.EasySearchASP.net Scott Cate

    Hey Scott — here’s a VS keyboard tip for you. I use this 100′s of time per day, and can’t live without it.

    Place your cursor next to a { or } (before or after, doesn’t matter.

    Then press CTRL+} and your curson will magical jump to the { or } sibling.

    Very nice huh!

    That being said, I still like your coding practice comments.

  • http://www.setfocus.com/CS/blogs/james_arendt/ James Arendt
  • Scott

    James, yeah it’s definately a little stinky. But some of the stinky is due to the way ASP.NET handles forms authentication and the fact that they just support the standard event args for the Authenticate event. The problem is I’m not following the arrow anti-pattern, I do have to check pre-conditions at each stage.

  • http://www.andrewconnell.com AC

    You need CodeRush by DevExpress.com. There’s a plugin that does this for you! It’s way slick.

  • http://www.codinghorror.com/blog/ Jeff Atwood

    Isn’t this just another case of reliance on ad-hoc patterns that really highlight shortcomings in the languge– eg, not all brackets are created equal?

    I’ll also second the arrow anti-pattern, although I think 2-3 and possibly even 4 layers deep isn’t THAT unusual.

  • http://www.munit.co.uk/blog/ Mun

    ReSharper displays the code at the opening bracket when the cursor is at the closing bracket, which helps a lot (and removes the need to use comments after the closing bracket) :-)

  • http://www.harriyott.com Simon

    The problem arises when maintaining the code. Changing a condition requires an additional change to the comment. In this case, if a future developer decides that it makes more sense to do the UrlReferrer check before the Querystring[pid] check and forgets to change the comment, then the comments suddenly become wrong, and really unhelpful.

    Five layers of nesting can become quite unreadable, so I would consider a couple of options for re-writing. Writing a method for each check that returns a boolean and using them in a single if statement may be appropriate (given that the opening statements haven’t been provided, and the comments hint that they are boolean checks), for example:

    if (IsHttpApplication(application) &&
    IsAuthenticated(user) &&
    CheckPid(Querystring[pid]) && …

    In C#, for example, if the first method returned false, then the others wouldn’t get called.

    Another alternative is to return from the method once a false condition is met. This flattens the structure, without changing functionality:

    if (IsHttpApplication(application) == false)
    {
    return false;
    }

    if (IsAuthenticated(user) == false)
    {
    return false;
    }

    I know someone will shout about having only one return point in a method, but with simple logic it isn’t confusing as long as there isn’t much else in the method. It’s also is easier to read than:

    if (IsHttpApplication(application) == false)
    {
    valid = false;
    }

    if (valid && IsAuthenticated(user) == false)
    {
    valid = false;
    }

    if (valid && CheckPid(Querystring[pid]) == false)
    {
    valid = false
    }

    if (valid)
    {
    // Do the stuff in the middle of the original ifs
    }

    Although this is still more readable than five nests.

    Hopefully the original was all ifs, had no elses, and only processed stuff in the middle of the fifth level. If there were loops, elses or processing on false conditions, then at least one method should be extracted.

  • http://www.brock.ac.uk/blogs/jon JonR

    i started doing this but, inexplicably, stopped. who’d have thought it – i’m lazier than lazycoder.