• (cs) in reply to Psyphyre
    Psyphyre:
    Never ask for permission to change bad code.
    Seconded. Don't talk to your boss about this, just improve it. If you have no reason (i.e. no open bug to fix), hide your improvement behind some other, maybe even unrelated, change request or bug report ticket.
  • Rohan (unregistered) in reply to bob

    I doubt someone dumb enough to write this piece of crap would even know what multi threading is :)

  • (cs) in reply to XIU
    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

    ReSharper for VS 2008 has given me nothing but problems. It causes my IDE to crash any time I open a find/replace dialog and seems to have completely destroyed my intellisense.

  • Haha (unregistered) in reply to s.
    apaq11:
    This reminds me of a similar chunk of code I found when looking through one of our projects

    Var = Var ? Var : True

    depending on language and compiler that may raise a warning about unintended side effects

    in C/C++ that would always evaluate to true and would copy the value of Var from Var to Var

    or.. was that the point of your post?

  • Boris (unregistered)

    It took a truth table to figure this out? It took me a couple of seconds to reduce this to return test, and that's after 5 hours of sleep and before the coffee kicked in.

  • Binks (unregistered) in reply to XIU

    Oh boy...that's just messed...

    So basically we first test if the variable is true.

    If the variable is true we then test if the variable is false. If the variable is both true and false we return false, otherwise, if the variable is both true and true, we return true.

    If the variable is false we then test if the variable is false. If the variable is both false and false we return false. If the variable is false and not false we return true.

    There are two parts of that line which are unreachable in standard boolean logic. For the first false to be reached test has to show as true for the first == and false for the second. For the second true test has to show as false for the first == and true for the second.

    I am in awe of the messed-up-ness of this code. Not only is it unnecessarily complex, but half of it is unreachable and the other half performs the same test twice (making sure?). It's a trifecta of bad code design!

  • lool (unregistered) in reply to ollo
    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...

    sth.? Are your o, m, e, i, n, and g keys broken?

  • _js_ (unregistered)
    if (test == true) {
      if (test == false) { // for quantum and DNA chipsets which allow multiple simultaneous states of variables
        return false
      } else {
        return true
      }
    } else {
      if (test == false) { // for quantum computers, the value might have changed due to measuring the value
        return false;
      } else {
        return true;
      }
    }
    
  • sth. (unregistered) in reply to s.
    s.:
    Heh, if it's not boolean you can use my favourite bool cast operator: return !!test;
    Heh, that's amazing. Heh, you know that? Heh, you know, the person right above you didn't just post the same thing. Heh, yeah, heh, you probably, heh, didn't just look at the last post and, heh, use it to make yourself look, heh, smart. Heh.
  • B (unregistered)

    Wouldn't this code return true in the case of a null value if test can be null? just returning test wouldn't have the same result as:

    return (test == false) ? false : true;
  • Anon (unregistered)

    If the boss doesn't know why some code is there, and it doesn't seem to be doing anything, then it's probably just better to dump it and don't tell the boss.

  • dignissim (unregistered)

    bComment = ((0-0) == 0)?0:1;

  • Ray (unregistered)

    The hideous truth is probably that someone found a race condition in the past, didn't understand the root cause, and added code like this because "it seemed to work", probably not even realizing that the compiler almost certainly optimizes this down to the same thing as suggested.

  • (cs) in reply to sth.
    sth.:
    s.:
    Heh, if it's not boolean you can use my favourite bool cast operator: return !!test;
    Heh, that's amazing. Heh, you know that? Heh, you know, the person right above you didn't just post the same thing. Heh, yeah, heh, you probably, heh, didn't just look at the last post and, heh, use it to make yourself look, heh, smart. Heh.

    The timestamps on the two posts are one minute apart. The first post was probably written while the second guy was writing his; I think we can cut him a little slack. Heh.

  • IV (unregistered)

    Okay, so he didn't change the code. Did he at least add a comment so that people looking at the code in the future didn't have to think about the convolution so long?

    Captcha: iusto

  • Adam Helps (unregistered)

    In Java, there's no excuse. It should be 'return test;'

    In C, 'return test' and 'return test ? true : false' are not quite the same. This is because, in C, boolean types are actually stored as 32-bit integers for speed reasons, and it's possible for a value other than 1 or 0 to get in there. The C definition of true is closer to 'anything that's not 0'. The simplest equivalent C code is probably return !!test;, which forces the value to 1 or 0.

  • Jay (unregistered)

    Back in my COBOL days I came across a program with three GOTO statements in a row, all to the same place. I asked the original programmer why he did that, and he replied that he wanted to make sure it really went there. Oh. I guess he only wrote one GOTO when he didn't really care if it went there or not.

  • Teh Mikeh (unregistered)

    If this were C# and test were a state-aware property, this code would do something different than just calling "return test" -- but it's more likely some perversion of java.

  • Ren (unregistered)

    This looks a lot like return (!!blah); in LPC, which isn't exactly the same as return blah; . With that you can make sure that whatever the other end gets is always 1 or 0. Not 200, "newt" or { 2:"blah", 1: "duh" }. It's actually a rather good way of handling polymorphics and transparency.

    Also, you never know when you absolutely need to change the constants(?) 'true' and 'false' into something else, in which case it might be important to always return exactly 'true' or 'false' and nothing in between. Or around. Or under.

    Why yes, I develop and maintain a huge system with loads of legacy code for a living, why do you ask?

  • The Doctor What (unregistered)

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

    Is not the same as: return test;

    Depending on the language. It would be true if the language was strongly statically typed and true was guaranteed a boolean (and boolean only had true/false, no NULL value).

    If it isn't strongly typed, then you need something like: return test == true;

    Of course, it also depends on the promotion rules of the language and whether == promotes things to boolean.

    If this was JavaScript, this may be useful: return (test === true) ? true : (test === false? false : test);

    Ciao!

  • Mykelyk (unregistered)

    The !!var is it's very important in javascript where not true are ''(""), 0, false, null and undefined;

  • will (unregistered)

    wow, im sorry, but my goodness some of you need a lesson in logic...

  • Max (unregistered) in reply to Randy A

    You'd think that MAYBE if it was in C that there was a macro evaluation reason behind it...

  • Mythprogrammer (unregistered)

    Proof that Java is not a bad language, just the good programmers are out numbered.

  • Scot (unregistered) in reply to Randy A

    Better to ask for forgiveness than permission when changing something like this.

  • Julio (unregistered) in reply to Randy A

    The guy probably just came out of university.. trying to use his acquired wisdom of logic.

  • Ullam (unregistered) in reply to TheRider
    TheRider:
    Psyphyre:
    Never ask for permission to change bad code.
    Seconded. Don't talk to your boss about this, just improve it. If you have no reason (i.e. no open bug to fix), hide your improvement behind some other, maybe even unrelated, change request or bug report ticket.
    Well, if you find such piece of code, you not only share it with your boss, also with the rest of the offoce and, how not, with thw dailywth.
  • (cs) in reply to Binks
    Binks:
    Oh boy...that's just messed...

    So basically we first test if the variable is true.

    If the variable is true we then test if the variable is false. If the variable is both true and false we return false, otherwise, if the variable is both true and true, we return true.

    If the variable is false we then test if the variable is false. If the variable is both false and false we return false. If the variable is false and not false we return true.

    There are two parts of that line which are unreachable in standard boolean logic. For the first false to be reached test has to show as true for the first == and false for the second. For the second true test has to show as false for the first == and true for the second.

    I am in awe of the messed-up-ness of this code. Not only is it unnecessarily complex, but half of it is unreachable and the other half performs the same test twice (making sure?). It's a trifecta of bad code design!

    You know what? I'm sick of this crap. Once again everyone has missed the point of the code. Just like in [url=http://thedailywtf.com/Articles/The-Alphabet-The-Hard-Way.aspx]The Alphabet The Hard Way[url] when the coder was prepared for a change in the alphabet. That is FORWARD thinking, people. Geez. This code is obviously preparing for the day when we actually venture through the looking-glass. When what's up is down and what's down is up. If that happens at the exact moment that expression is being processed, then we're covered. I know I'm going to start taking these precautions. So screw you guys. I'm going home.

    Addendum (2008-04-04 17:14): When hyperlinks attack! Someone end the url!

  • SJS (unregistered) in reply to Binks

    When folks come into an IRC channel and demand for someone else to write some code for them (happens all the time), or worse, resort to vague threats and name-calling if someone doesn't give them a code "example" RIGHT THIS MINUTE, this is exactly the sort of thing that often gets kicked back to 'em with a sweet smile and a kind word.

  • CynicalTyler (unregistered)

    Obviously this is meant to protect against concurrent modification of a publicly available static boolean. If you just have "return true;" and I go to return to one thread when the value is true, but some other thread comes in and changes it to false before I can finish returning, then the whole application explodes! Hence the multiple checks. No WTF here...

  • (cs)

    That is one ingenious way of filtering out the mysterious third boolean value, FileNotFound. Brillant!

  • xoc (unregistered)

    You do realise, of course, that this is exactly the reason why Vista is such a steaming pile.

  • Rich (unregistered)

    Yes, but its important to have this kind of logic in a multi-threaded environ when the value of test could change at any time! This is clearly high-quality thread-safe code, ruined...

  • Robin Goodfellow (unregistered) in reply to Randy A
    Randy A:
    They finally posted it...

    To answer some questions in the comments:

    The language was Java test was nothing more than a boolean Supposedly it was written when the author was learning Java

    And yes I've since learned not to 'ask' about changing bad code.

    Never ask about truly bad code. If you know 100% for sure that the code is bad, then change it. Asking indicates to your boss that you aren't 100% sure if it's the right thing to do, in most situations your boss is much less likely than you to have expertise and be able to know for sure whether changing the code is good. Meaning, your boss is more than likely to err on the side of caution. Whereas if you told your boss or they found out about the change and you said with complete confidence that it was a good change, you'd be much less likely to get resistance.

  • Uri Schonfeld (unregistered)

    Could it be a sanity test for the CPU?

  • David (unregistered) in reply to Jay

    Back in my (shudder) COBOL days, I had to use a compiler that didn't quite understand PERFORM statements all the time. We reported it to the compiler writers, and they replied, "Yup, that's a bug." I have sympathy with anybody who wants to emphasize something to a COBOL compiler.

  • Zero (unregistered)

    It might get confused and return maybe... poor computer.

  • gus (unregistered)

    When you need to ask your boss permission to edit code and he doesn't trust you, it's time to quit, even if it's because you don't know what the heck you're doing. In effect it'd return defined true or defined false vs. all the myriad of possible values that also legitimately test as true or a false value like NaN, it can fix issues in bad upstream code like testing return_value == true, and NaN can confuse; a==a is false when a is NaN for example. However even if you think you need this have these guys never heard of macros? Geeze.

  • Lee (unregistered)

    I once saw a shell script around 20 lines long that was a series of 'head -1' and 'tail -1' lines each of which redirected to a different temporary file, each reading the output from the previous command. I eventually figured out that it was just removing the top 5 lines and the last line, and simplified it to this... sed '1,5d;$d'

  • shash (unregistered)

    I've seen enough of this from newbies...

    if ( cbSomething.Checked == true )
    	return true;
    else
    	return false;
    

    They usually manage to optimize the else to not use an else-if...

  • Mania (unregistered) in reply to DaveK
    DaveK:
    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[*]. [*] - after folding, of course.
    Ok, first one I tried. Borland C++, latest version (I believe) poorly optimises !!. I assume all earlier versions have the same flaw too. Right there is "a lot of compilers". Yes I know Borland make terrible optimizers, and I won't disagree that right there is a good reason to not use them. But nonetheless, their products are widely used. And in any case, I still think != 0 is clearer, which was my main point.

    return b?1:0 test eax, eax jz <addr> mov eax, 0x00000001 jmp <addr2> xor eax, eax return !!b cmp eax, 0x01 sbb eax, eax neg eax cmp eax, 0x01 sbb edx, edx neg edx mov eax, edx return b != 0 test eax, eax setnz al and eax, 0x01

    So what do I win?

    As for the explicit cast, I agree completely. Best way to handle it. However it doesn't work in C.

  • erayd (unregistered) in reply to apaq11
    apaq11:
    Var = Var ? Var : True
    That bit makes sense though - it means "If var is a loosely-typed value equating to true (e.g. a string or something) leave it alone, otherwise set it to boolean true".
  • Mania (unregistered) in reply to Mania

    Apologies, I forgot that leading white spaces are removed. (Yes I know about non-breaking spaces, but still a lot of forums allow leading spaces - don't challenge me on that one I can't be bothered proving myself again =p)

  • requiredname (unregistered)

    Note that while the whole thing is stupid, return (test == false)?false:true

    is not equivalent to

    return test

    because the first only returns values false and true, while the second may also return other values. While things having to cope with them will normaly consider non-0 (i.e. non-false) values to mean true, it's a not a normalized value, which could cause problems with other dirtry tricks around. (like using a bool as array index and other things)

  • Jared Parsons (unregistered)

    I more commonly run across the following

    if ( test ){ ... } else if ( !test ) { .... } else { // Gotta love it. }

    The worst case was during a larger code review. The else block called a function we were changing. During the process of reviewing the older use of the function I found several other WTF's inside the else block. Unfortunately the if/else if blocks were so large they were off screen and I spent quite a bit of time understanding this code before I reliazed there really was no point.

  • Iago (unregistered) in reply to Mania
    Mania:
    DaveK:
    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.
    Ok, first one I tried. Borland C++, latest version (I believe) poorly optimises !!.
    Fair enough. On the other hand, the two I tried (gcc and Intel's icc) both generate exactly identical code for the two, even if all optimisations are explicitly disabled with -O0.

    In any case, this kind of micro-optimisation is unlikely to significantly alter efficiency even of programs compiled with Borland C++. The decision should be made based on which construct is more likely to be understood by future maintainance programmers, not on which is "faster", unless you've actually benchmarked the code and found that this is a genuine bottleneck!

  • Villane (unregistered)

    I pasted this snippet into Eclipse to see what "quick fixes" it would offer and since it didn't offer to simplify it, I determined it was already in it's simplest form. It did offer helpful things like "replace conditional with if-else" and "invert if". Using those I architected this much more readable refactoring:

    	if (false != !test) {
    		if (!test != true) {
    			return true;
    		} else {
    			return false;
    		}
    	} else {
    		if (!test != true) {
    			return true;
    		} else {
    			return false;
    		}
    	}
    
  • iMalc (unregistered) in reply to Randy A
    Randy A:
    And yes I've since learned not to 'ask' about changing bad code.
    I hope that means you fix it anyway.

    Those who should make decisions in the interest of maintainability of code, are those who have to actually maintain it.

  • (cs) in reply to dpm
    dpm:
    Actually, a true shortened form would be

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

    correct because this will correctly handle the situation where test == fileNotFound

  • Tirinoarim (unregistered) in reply to Jay

    Of course theres the 3-value-boolean problem.

    In certain languages (gee thanks MickeySoft) a boolean can be true, false or null. In which case

    if (test) do(a) else do(b)

    does neither!

Leave a comment on “The Test of Truth”

Log In or post as a guest

Replying to comment #188176:

« Return to Article