• (nodebb)

    Grand classic that I ran across one time:

        /* This does not work, and I don't know why not. */

    I dug through the code around the comment for about ten minutes, and worked out the exact code path that went wrong, and that it was a common case.

    Sharing this revelation didn't win me any friends.

  • (nodebb)

    That isValid one makes my teeth itch, mostly because the returns that are a definite answer (true and false) are inverted.

  • (nodebb)

    Would love to see the transcripts of the code reviews <<ducking and running>>

  • Andrew Miller (google) in reply to Steve_The_Cynic

    The comments to isValid say the logic is inverted. I wouldn't like to bet on the code actually implementing that....

  • (nodebb)

    Am I getting my order of operations confuzed, or is Chris' example a complicated way to calculate 1/5 of getHeight()?

  • Robin (unregistered) in reply to Steve_The_Cynic

    I thought the same, although I'm also sad they used null instead of FILE_NOT_FOUND for the third Boolean value.

  • yeah but no but (unregistered)

    Code and a comment that survives untouched since 2010-04-29T02:46:46Z:

    $modules[$module['module']] = $module; unset($modules[$module['module']]['module']); // Woooo ....

  • Michael R (unregistered) in reply to Steve_The_Cynic

    You must be new here.

  • (nodebb) in reply to Robert_Morson

    getHeight() might have side effects that necessitate it being called twice before calculating one fifth of it.

  • (nodebb) in reply to Steve_The_Cynic

    I was hoping the error is in the comment, missing the 'in' of 'invalid'. Then it makes sense.

    1. Can't determine for Reasons - null (although raising an exception would be better).
    2. Determine it's invalid - false.
    3. Otherwise - true.

    There are, of course, arguments to be made for trying to actively test the location thoroughly for being valid, and if it is return true, else false. But then we're talking about locations, which might mean addresses, and that way lies madness.

  • xorium (unregistered) in reply to yeah but no but

    Who would dare to touch such a line?

  • (nodebb) in reply to Andrew Miller

    I don't care what the comments say. As described, you end up with e.g.

        if ( isValid(...) )
            // handle the case where it's valid

    which I would read as "if NOT is valid, handle the valid case".

    Addendum 2022-09-01 11:02: Ugh. if ( !isValid...

  • (nodebb) in reply to Athanasius

    The function is called isValid but returns false for valid. That's top-tier RWTF territory right there.

  • Conradus (unregistered) in reply to yeah but no but
    $modules[$module['module']] = $module; unset($modules[$module['module']]['module']); // Woooo ....

    Buffalo buffalo Buffalo buffalo buffalo buffalo Buffalo buffalo

  • (nodebb) in reply to Jeremy Pereira

    getHeight() might have side effects that necessitate it being called twice before calculating one fifth of it.

    So TRWTF is whatever code lives in getHeight() ? Sad; we'll never know :(

  • Gonk (unregistered)

    My favourite comment of all time: // Comments removed for clarity

    The sad part of that was it was probably very true. Management had changed from a 3 level to a four level classification hierarchy and insisted that it be implemented by splitting the mid-level identifiers in two but calling the top level something new. But all the comments of course had the names of the old hierarchy. Of course the real WTF was management messing with the implementation details... He had been a programmer years before and thought he still was..

  • PonyBoy (unregistered)

    Is NULL better than FILE_NOT_FOUND?

  • MRAB (unregistered) in reply to Robert_Morson

    You're forgetting about integer division: 50 / 5 == 10, but 50 / 100 * 20 == 0 * 20 == 0.

  • Vivek Thakur (unregistered)
    Comment held for moderation.
  • Ogre (unregistered)

    My favorite comment that I came across stated "This is where the miracle needs to happen." Given how buggy that function was, I guess there was no miracle.

  • Aussie Susan (unregistered)

    many years a go I was using a '4th generation' programming language (Rally) that let you create 'short' functions - they were supposed to be no more than a few 10's of lines long at most. I came across a function written by an 'external contractor' that was over 4000 lines long. There was only 1 comment and that was at the top: "This code is like a rat's ar*e i a whirlwind but don't touch it - it works" Of course it didn't! Susan

  • Loren Pechtel (unregistered)

    As for the getHeight() one--I've seen issues where the side effect is actually lazy instantiation of something deeper down and you get bogus data on the first call if your first call is not what was expected to be first. His workaround won't fix that, though.

  • (nodebb)
        * If not do not? */
    s = !package.isNotValid()&&!package.isNotAvailable();
    if (!s)
        package.sendError("Package is invalid or unavailable);
    return s;

    Addendum 2022-09-01 23:16: Sorry, typed from memory, typo not in original I am sure.

  • Conradus (unregistered) in reply to BatConley
    Comment held for moderation.
  • MaxiTB (unregistered)

    In which crappy language can this be ever null?

    Boolean isValid(String location);

  • Matthijs van Duin (github)
    Comment held for moderation.
  • (nodebb) in reply to MaxiTB

    If this is Java, then the return value of a function that returns an object (including the built-in Boolean class (as opposed to the primitive boolean)) is nullable. https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/lang/Boolean.html for reference.

    Addendum 2022-09-02 18:05: To rephrase, hopefully more clearly: in Java, objects are nullable, including objects that are instances of the built-in Boolean class.

  • (nodebb) in reply to Steve_The_Cynic
    The function is called isValid but returns false for valid.

    No, that's only what the comment says the function returns.

  • (nodebb)

    This is my favorite from a professor when I was in college. He had to work on a giant file of Assembler code that only had one comment in the entire file: "Turn the bugger on". He never did figure out what "the bugger" was or what was being done in the code that might turn it on.

  • Neveranull (unregistered)
    Comment held for moderation.
  • Diane B (unregistered)
    Comment held for moderation.
  • Gurdeep Singh (unregistered)
    Comment held for moderation.
  • Chatogaster (unregistered) in reply to Steve_The_Cynic
    Comment held for moderation.
  • (nodebb)
    Comment held for moderation.
  • Trupik (unregistered) in reply to Dragnslcr

    "the bugger" is clearly the opposite of "the debugger"

  • Mark (unregistered)
    Comment held for moderation.

Leave a comment on “Latrinalia”

Log In or post as a guest

Replying to comment #:

« Return to Article