Comment On The Test of Truth

Anyone else notice Gilroy in the image attached to the article linked at the beginning?

What language is this in?

Perhaps a loosely-typed language (PHP?), where test could be sth. other than true/false/file_not_found? Then there might be a fraction of sense in these lines...

ollo:

What language is this in?

Perhaps a loosely-typed language (PHP?), where test could be sth. other than true/false/file_not_found? Then there might be a fraction of sense in these lines...

Probably C# or Java...

Addendum: Since some of those tests can be proved to fail or pass (e.g. if you've tested a condition to be true, then an immediate test for false can never succeed) would a decent compiler could remove some of the redundant logic....unless the code was multithreaded allowing the value of test to switch values between the first and subsequent conditional. Maybe that's what the original coder was intending; the code is double-checking the return condition in a multithreaded program and the author never heard about atomicity.

The language could support properties, as such the blah.variable could actually be a method call. And who knows what that method call is doing. Maybe it only returns the correct response every second execution?

took a contact

Hmm. That would annoy me. :)

ED: Fixed!

Actually, a true shortened form would be

if (test == false) return false;
return true;

I vote for "the programmer is just an idiot."

bob:

Maybe that's what the original coder was intending; the code is double-checking the return condition in a multithreaded program and the author never heard about atomicity.

That would be quite astoundingly bad. Anything that would rely on such a devious practice is even more WTF-y than this fine snippet appears to be otherwise. What's doubly nasty is how the "pattern" is repeated; that's a sign of idiocy at work. Abstraction is required; preferably of the code and the programmer^Wperpetrator...

You would have to return !test and then update the calling code to reverse the logic.

Assuming that "test" could hold a third value (like 'null'), then false returns false, and true and anything else returns true. Of course, it would help to know the language and what type of variable "test" is...

This logic is not covered by "return test".

At least the operator== is not overloaded. Is it?
And test is of type boolean?

A reasonably tidy way of resolving possibly-true-or-false things to a nice binary is
return !!test;

Heh, if it's not boolean you can use my favourite bool cast operator: return !!test;

...Randy's boss replied "I'm not sure why it's in there, and it's probably best we keep it in there."...

The term cargo-cult programming comes to mind :-)

Well, the code depends on what type 'test' is. If test was an instance of this:

class T
{
public:
bool operator == (bool rhs)
{
bool result = rand () < (RAND_MAX >> 1);
return result;
}
};

then you'd get really random true / false values because calling rand more than once increases the randomality.

Skizz

bob:

Anyone else notice Gilroy in the image attached to the article linked at the beginning?

That's because the same guy that did the codethulu image also does MFD...

This reminds me of a similar chunk of code I found when looking through one of our projects

Var = Var ? Var : True

You guys are missing the REAL reason. The original programmer is an evil alien trying to drive all human programmers insane thus making the Earth ripe for invasion by intelligent computer viruses.

BWAHAHAHAAHAHAHAHA

Kluge Doctor:

Assuming that "test" could hold a third value (like 'null'), then false returns false, and true and anything else returns true.

What's this 'null' you speak of? Is it anything like File Not Found?

Never ask for permission to change bad code.

bob:

Addendum: Since some of those tests can be proved to fail or pass (e.g. if you've tested a condition to be true, then an immediate test for false can never succeed) would a decent compiler could remove some of the redundant logic....unless the code was multithreaded allowing the value of test to switch values between the first and subsequent conditional. Maybe that's what the original coder was intending; the code is double-checking the return condition in a multithreaded program and the author never heard about atomicity.

Hmm, but what if test is a property in the current object, whose get function is stateful?

jgayhart:

took a contact

Hmm. That would annoy me. :)

It gets worse... Apparently the poor guy also started into the heart of the Great Codethulhu with his eyes or something.

Psyphyre:

Never ask for permission to change bad code.

I kind of agree... The guy you're asking may have been it's author :p

XIU:

I just tried this in C# with test just a regular bool and ReSharper simplified it to this:

return test ? (true) : (false);

The suggestions were:
- Comparison with true is redundant
- Expression is always false
- Expression is always true
- Code is unreachable

Too bad it didn't refactor completely to return test; but at least you know then what it does :P

If test can have more than a true/false value; that's as far as you can go without changing any possible outcome. Only if you are sure test will be either true or false can you whittle is down to 'return test'... and since this is only a representative line, we don't know if that's the case (but we'll assume Randy A could've determined that)

There is no way someone could have come up with that code by being a bad programmer. A vindictive one perhaps...

This is not PHP (uses $ sign for variables!)

So if the first evaluation (test == true) is true it will commence the second evaluation ((test == false) ? false : true) and return true.

But that is redundant since we already evaluated test to be true

If the first evaluation is false, then it does the third evaluation (test == false) ? false : true) and return false.

IMHO this double test is redundant even if its a non bool variable! I would just do

return (test != null) ? test : false;

s.:

Heh, if it's not boolean you can use my favourite bool cast operator: return !!test;

I prefer: return test != 0;

It generates better code on a lot of compilers, and its intent is clearer. You'd have to be a fool to "optimize" it to just return test, where as a !! could easily be accidently "optimized".

Let me guess - the original programmer was either extremely bored and did this for the kicks of it, or he did it for "job security" obfuscation reasons :)

If test's type has evil operator overloadings, the most you can reduce this to is:

test == true;
return ((test == false) ? false : true);

That is, assuming the language doesn't allow for ?: overloading.

Maybe the programmer tried to avoid to accidentally return FILE_NOT_FOUND.
But on the other hand, is there any case where you might want to avoid FILE_NOT_FOUND to be returned? I don't think so.

SketchySteve:

There is no way someone could have come up with that code by being a bad programmer. A vindictive one perhaps...

The BCFH - Bastard coder from Hell?

f0dder:

Let me guess - the original programmer was either extremely bored and did this for the kicks of it, or he did it for "job security" obfuscation reasons :)

My guess is that author of this code wrote it on April 1st. It is too bad that this wasn't posted a week ago. I might have checked in some code Tuesday. :-)

Mania:

s.:

Heh, if it's not boolean you can use my favourite bool cast operator: return !!test;

I prefer: return test != 0;

I like "return (bool)test;"

Mania:

It generates better code on a lot of compilers,

No. No it doesn't. Not this millennium, anyway. I challenge you to find one compiler anywhere that doesn't generate identical code for the two. In fact, I challenge you to find one compiler anywhere that doesn't translate the two different ASTs into the exact same intermediate representation[*].

Mania:

and its intent is clearer. You'd have to be a fool to "optimize" it to just return test, where as a !! could easily be accidently "optimized".

I don't see why anyone who thinks "!!" is superfluous and can be optimised away wouldn't also think the exact same about "!= 0". After all, everyone "knows" that the truth-value is the same as the non-zero status of a variable so testing for non-zero is a no-op. But then, like half the posters here, they make the mistake of thinking that "== true" is the same as "!= false", which is only guaranteed for a real native boolean type, and not the integer-status-treated-as-a-bool that so much C code uses.

That's why an explicit cast is clearer than either "!!" or "!=0"; it makes clear that you aren't really interested in doing a test or comparison, you're making sure that what you have really is a bool.

[*] - after folding, of course.

Assuming that test is a nullable bool you could use the null coalescing operator "??", which returns the value to the left if it is not null, and the value to the right if it is.

return test ?? false;

Does Randy ask his lead's permission every time he changes a line of code? Sounds inefficient.

apaq11:

This reminds me of a similar chunk of code I found when looking through one of our projects

Var = Var ? Var : True

Well, likely they decided it's better to be divided by truth than by lies.

equex:

This is not PHP (uses $ sign for variables!)

define('test', rand(0,1));

What is it with this crew and booleans?

Sooooooooo true. I dump junk code on a regular basis when I find it. Unfortunately, other 'developers' I know of aren't so enlightned.

A friend was writing some subsystem in PHP some time ago.
Thing is the subsystem could cause many different faults which should be handled differently, and the system above it would launch only one error handler and only if the subsystem returned 'false' (and it wasn't permitted to change the system, only the error handler).

So my friend designed an ingenious method of encoding error codes = integers inside 'false'.

array(false) = 0
array(array()) = 1
array(array(array())) = 2
array(array(array(array())))) = 3
...
array(array(),false) = 10
...
array(array(array()),array()) = 21


The error would be dispatched because they all cast to bool return false, but he could extract what was returned from the procedure, identify the error code by the structure shape and react properly.

We thought, what about encoding ASCII error messages that way.

Aw, wotta wimp! He asked permission to change it, fer cryin' out loud... you're a maintenance developer, man! Here's the definition of "maintain": "To keep in an appropriate condition, operation, or force; keep unimpaired." All right, it's impaired; fix it! That's your job. Does a maintenance engineer (formerly "janitor") ask permission to mop up a spill in the hallway?

I encounter it every day. As well as stuff like this:

if (string == null || "".equalsIgnoreCase(string)) {
return false;
} else {
return true;
}

Good, though, that in our company you do not have to ask a manager before making the world a better place.

XIU,

WTF? You need a tool to tell you that

return boolean

is equivalent to any logical obfuscation that returns 'true' if the boolean is true and 'false' if the boolean is false?

...and your tool couldn't even do it! Language doesn't really matter.

ollo:

What language is this in?

Perhaps a loosely-typed language (PHP?), where test could be sth. other than true/false/file_not_found? Then there might be a fraction of sense in these lines...

No, there still wouldn't be. If there were other values that test could contain, the if statement should have been testing for them as well.

You can't justify this - it's a WTF.

bob:

Addendum: Since some of those tests can be proved to fail or pass (e.g. if you've tested a condition to be true, then an immediate test for false can never succeed) would a decent compiler could remove some of the redundant logic....unless the code was multithreaded allowing the value of test to switch values between the first and subsequent conditional. Maybe that's what the original coder was intending; the code is double-checking the return condition in a multithreaded program and the author never heard about atomicity.

Nope. You still can't justify it. It's a WTF.

Of course, it's no surprise that the "code" (using the term loosely) author didn't know about short-circuit boolean evaluation.

snoofle:

What is it with this crew and booleans?

It is only because people don't understand the true nature of a boolean.

Booleans are only ever bi-valued, true or false.
Boolean variables can have the possibility of having a null value.
Boolean variables with a null value should be safely assumed to be either true or false.
If a boolean variable with a null value can not be safely assumed true or false, you don't have a boolean, but a tri-state enumerator, so don't confuse things by attempting to handle it with a bi-state representation.

Addendum (2008-04-04 10:14):
Oh and the joke illustrates that without saying this specifically.

Mania:

I prefer: return test != 0;

It generates better code on a lot of compilers, and its intent is clearer. You'd have to be a fool to "optimize" it to just return test, where as a !! could easily be accidently "optimized".

If the compiler you're using in this day and age still requires that you do these kind of optimizations, it's time you got out of the '80s. The 21st century is waiting for you.

http://www.jetbrains.net/jira/browse/RSRP-63747