• Rob (unregistered)

    I hope origional is spelled consistently throughout the code, or there will be other problems.

  • (nodebb)

    if(frist) ;

    IOW, even the comment is empty. :)

  • (nodebb)

    More useful comments (I hope) ...

    I almost wonder if the first one if (!checkAndDelete( ... isn't a warped attempt to call a method that has a return value and swallow the return value. Yes, darn near every language nowadays will swallow a return value. In Ye Olden Tymes that was less true. Of perhaps the if(...) construct was a way to stop a warning or lint message about an unused return value. But that doesn't explain the useless negation.

    Of course the real truth is probably that there used to be a body under the if, then requirements changed. But the method call in the if predicate has side effects, so of course it needed to stay. Under the misguided principal of "minimum possible changes ever; refactor never" they just removed the body leaving the if (...) exactly as it was.

  • Robin (unregistered)

    I can guess how the first one happened. Presumably checkAndDelete both does the deletion and rerurns a value representing the result of the check (I guess whether there was anything to actually delete?). At one point, this return value was used to conditionally run some extra code - and at some point, that additional code became unnecessary.

    At that point, some less bright team member presumably first tried deleting the whole block, but found the code broke (the side effect of the deletion being important). So they decided to keep the if and just remove the body - as opposed to the less confusing alternative of just leaving the checkAndDelete call as a single statement, not caring about its return value.

  • Robin (unregistered)

    Aargh, the post above appeared while I was typing. Still, great minds and all...

  • Richard Yates (unregistered)

    "$mail->SMTPAuth = false; // turn on SMTP authentication"

    This is certainly from PHPMailer a widespread email utility for PHP. I probably even have this exact line in my testing server copy of the code. The value gets set to "false" or "true" depending on how code is being tested.

    Sure, the comment (which comes that way with the PHPMailer package) should be "// turn SMTP authentication on or off", but that's pretty picky when the point is so obvious to anyone actually using the code.

  • Foo AKA Fooo (unregistered)
    Comment held for moderation.
  • Tinkle (unregistered) in reply to Richard Yates
    Comment held for moderation.
  • Argle (unregistered)
    Comment held for moderation.
  • Duke of New York (unregistered)

    If you can code — and not make your code your master; If you can comment — and not make the comments a shame; If you can bear to have your flag for authentication Set by knaves in mockery of the same; Empty line

  • (nodebb)

    Sure, the comment (which comes that way with the PHPMailer package) should be "// turn SMTP authentication on or off", but that's pretty picky when the point is so obvious to anyone actually using the code.

    Be careful with this rationale. If "it doesn't matter because it's obvious", then what's the rationale for doing it at all? That's actually what everyone is trying to say when they say that the comments shouldn't simply repeat the code. If all you have to say to the next person that reads this code is what it already says... then don't say it again. Leave the comment out.

  • Chris (unregistered)
    Comment held for moderation.

Leave a comment on “The Saddest Words: What If”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article