- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Admin
Sounds like debugging code that wasn't meant to be checked in.
Admin
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 thisif
was never called and the1==2
just makes it more obvious (granted, it's still a WTF to not just delete the inactive code).Admin
Exactly my first thought. I've seen plenty of these. That's why I always check what I'm checking in.
Admin
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".Admin
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.)
Admin
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.
Admin
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.
Admin
if (!$path) {
is also bad because it's not just checking whether$path
isfalse
, but whether it isfalsey
, which is not the same thing.For example, if
$path
has the value "0", then theif
branch will be executed. Because PHP.Admin
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.
Admin
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 theelse
block is long and does real work. It's harder to read if there's a long distance from theif
to the correspondingelse
, 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).
Admin
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 andreturn 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 returnfalse
on error. But even that is not always enough, as some functions likestrpos()
can return a falsey value like0
without it being an error, so for those functions you need to explicitely checkif ($result === false)
Anyway, be careful about using return values to handle errors in other cases than built-in functions.
Admin
Can truth and falsehood exist independently of a statement that is true or false? A question that puzzled the ancients and still puzzles coders today.
Admin
That was my immediate thought. They forced a code path and forgot to revert.
Admin
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.
Admin
if ~fieldIsEmpty(field) {/do some complicated stuff/} else {/throw a simple exception/}
Admin
Developer forgot to remove a debug line.
Admin
'But no, we did none of that. Now we just have a mystery, lingering in the code, waiting for future developers to stumble across it and ask, "WTF?"'
So, this code is not a WTF but a WTF generator?
Admin
TRWTF is failing to account for FILE_NOT_FOUND
Admin
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.
Admin
One of my coding pet peeves are developers that leave huge swaths of commented out code when using a source control system.
Admin
@Robin: i'd like to present to you a different approach, to emphasize readability: put the unhappy path first (if they're not considered balanced, as in this code exampe, as this is 90% of if-then-else), as those usually are much shorter and readers can much more easily skip over it than finding
$wtf_amount
else-clauses of error handling at the bottom of the file (we all have seen this, pure puke)Admin
TRWTF is that this passed through code review (or that there was no code review).