• Zatapatique (unregistered)
    if (frist == secnod) {
    	//do stuff
    } else {
    	//do other stuff
    }
    
  • (nodebb)

    Sounds like debugging code that wasn't meant to be checked in.

  • (nodebb)

    The first version of if (!$path) { is a WTF to me, too. What does $path contain? Judging by the name, it's likely a string with a path to something. How can that ever be true? Maybe if $path is null (or whatever it is in PHP), but I'm not even sure that's how PHP works (and I'm not spending my time to research that). Frankly, my first reaction is, the "then" block of this if was never called and the 1==2 just makes it more obvious (granted, it's still a WTF to not just delete the inactive code).

  • Vilx- (unregistered) in reply to Medinoc

    Exactly my first thought. I've seen plenty of these. That's why I always check what I'm checking in.

  • (nodebb) in reply to Mr. TA

    Non-null, but empty, strings are false in PHP. If memory serves, "0" is also false. It's probably just saying "if we don't have a path".

  • Robin (unregistered)

    While it's admittedly a non-issue compared to what it was changed to, I have a near-irrational but deeply-felt antipathy to if (!something) { /* some code */ } else { /* some different code */ } constructions. If we're catering for a condition being either true or false, please for the love of whatever you consider holy, put the "true" case first!

    (Before you ask, I exempt early returns from this, for obvious reasons - and they're not strictly if/else anyway.)

  • (nodebb)

    A minor annoyance: it always tickles me the wrong way if you have an else clause, with a negation in the condition. Probably just me.

  • (nodebb) in reply to Domin Abbus

    It's not just you. I've encountered at least slightly more people online who agree with you, and slightly more people in real life who prefer to order the if and else based on some aspect of the code or the expected behavior. More common case first; more common case second; longer block first; longer block second (which is often what I find easiest to read)...

    When I was a young warthog, I used to write them in whatever order I thought through the behavior I needed, and would feel decidedly unnatural about changing it later.

    I think I came across one extreme case where someone was so dead set against negations like this in the condition that he would sometimes write an empty if block rather than use the negation and not have an else.

  • Sauron (unregistered)

    if (!$path) { is also bad because it's not just checking whether $path is false, but whether it is falsey, which is not the same thing.

    For example, if $path has the value "0", then the if branch will be executed. Because PHP.

  • Ternary King (unregistered)

    I hope you actually asked the colleague in question "wtf" and got an answer. My guess is an accidentally committed debugging attempt, especially if the commit message wasn't relevant.

    @Robin/Domin - For me, the semantically interesting case should come first. "!$path" - a validation condition - is a reasonable thing to use in the if branch. This one would be ok either way around for me. I mean, except that content in directories starting with a 0 won't be processed properly; using boolean logic on strings in languages that allow that (PHP or JS/TS) can result in subtle WTFs later on.

  • (nodebb) in reply to Robin

    While I generally agree with you, and I love the command in my IDE to swap if and else, putting the negated case first is sometimes a little more readable. This is usually when it's an error case, like if (!$result) and the body is just one or two lines to report the error or set an error code, while the else block is long and does real work. It's harder to read if there's a long distance from the if to the corresponding else, because it's harder to see how they line up.

    But many programmers prefer to solve this problem using else-less programming (essentially what you allude to when you refer to early returns).

  • Sauron (unregistered) in reply to Barry Margolin

    This is usually when it's an error case, like if (!$result)

    I'm wary of your example.

    I have some colleagues that like making the functions return true; when things go well, and when things go wrong they make it swallow every exceptions and return false; . And that's an absolutely shitty idea, because it destroys useful information about the nature or context of the exception. I hope this is not the kind of WTF you're trying to promote here.

    It is true that checking if (!$result) may in some narrow cases be useful, as some built-in functions return falseon error. But even that is not always enough, as some functions like strpos() can return a falsey value like 0 without it being an error, so for those functions you need to explicitely check if ($result === false)

    Anyway, be careful about using return values to handle errors in other cases than built-in functions.

  • Duke of New York (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to Medinoc

    That was my immediate thought. They forced a code path and forgot to revert.

  • (nodebb) in reply to Domin Abbus

    Fortunately there are some linters for various languages that agree with you and me on this. On the basis that when you get to the else you're mentally thinking 'i shall do this if not not some condition' and it hurts my brain.

  • (nodebb) in reply to Robin

    if ~fieldIsEmpty(field) {/do some complicated stuff/} else {/throw a simple exception/}

  • (nodebb)

    Developer forgot to remove a debug line.

  • richarson (unregistered)
    Comment held for moderation.
  • löchlein deluxe (unregistered)
    Comment held for moderation.
  • okkero (unregistered)

    TRWTF is failing to account for FILE_NOT_FOUND

  • SG (unregistered)

    Hardly a WTF, as these things go.

    It's recent, you know who did it, so you ask them. And either it was an accident and they fix it promptly — or it was deliberate, so you beat them to death and have their head mounted on a spike. Happens all the time.

  • IfnotNotfalseElsenotTrue (unregistered) in reply to Domin Abbus
    Comment held for moderation.
  • Chris O (unregistered)

    One of my coding pet peeves are developers that leave huge swaths of commented out code when using a source control system.

  • unhappyPathFirst (unregistered)
    Comment held for moderation.
  • Craig (unregistered)
    Comment held for moderation.

Leave a comment on “A Poorly Pruned Branch”

Log In or post as a guest

Replying to comment #:

« Return to Article