3

I have inhereted some code and I see somthing curious in there:

if (true == someFuncThatReturnsBool())
{
   // Do somthing
}

bool someFuncThatReturnsBool()
{
    bool retVal = false;
    // .... do some stuff

    return retVal;
}

In the "if" statement they have used true == someFuncThatReturnsBool() as a boolean expression, where for boolean values I would normally do:

if (someFuncThatReturnsBool())
{
   // Do somthing
}

What, if any, is the difference to the generated code? Is there any advantage to using the "true ==" notation, other then perhaps for some clarity that the function returns a boolean?... type checking??, personally I can't see any advantage...

Thanks :)

code_fodder
  • 15,263
  • 17
  • 90
  • 167
  • 1
    You are correct. There is no advantage, just wasted and redundant syntax, and possibly pointless extra bytecode as well. Somebody didn't understand the language. Why stop at ('true == ...)`? Why not `(true == (true == ...))`? – user207421 Aug 29 '13 at 10:58
  • @EJP lol, thanks!... this is what I thought, but when I see someone has done somthing I "don't get" I figure its worth asking about it ... like, what did they know that I don't! :) – code_fodder Aug 29 '13 at 11:09
  • @MikeSeymour ah, if so I do appologies, I did look but I did not think to actually add the code in the title! - so I missed that one – code_fodder Aug 29 '13 at 11:40
  • @code_fodder: No need to apologise; it's not something that's easy to search for. Duplicates aren't (usually) deleted, and can make searching easier in the future. – Mike Seymour Aug 29 '13 at 12:15

5 Answers5

5

What, if any, is the difference to the generated code?

none.

Is there any advantage to using the "true ==" notation, other then perhaps for some clarity that the function returns a boolean?

IFF the function is named poorly then maybe I would write the explicit version. Otherwise, if it is apparent from the function that it is a boolean check (i.e. is_XXX fashion), then there is no reason to use it.

Jesse Good
  • 50,901
  • 14
  • 124
  • 166
  • 2
    If the function is named poorly, I'd change its name. Using some twisted notation like `if ( true == function() )` is not going to make things more readable. – James Kanze Aug 29 '13 at 11:03
  • @JamesKanze: I would change the name also, but then that is not always possible. The readability part could be debatable though. – Jesse Good Aug 29 '13 at 11:10
  • Thanks for the affirmation :) @JamesKanze "twisted"....lol – code_fodder Aug 29 '13 at 11:10
2

In my opinion, there can be be a small readability advantage if that someFuncThatReturnsBool functions name reads like it's just some value, not particularly a boolean. Thousands (perhaps most people) would call me an idiot for that.

Normally, boolean variables and functions would be named is_whatever or has_whatever. So normally, an if statement reads as if (is_whatever) or if (has_whatever). If I don't see that, or of course a relative operator, it seems a bit smelly to me.

But sometimes boolean variables and functions don't use that name formula, especially if the boolean type is specified via a template parameter. In that case, I might well use if (whatever == true). Or, using Yoda conditions, that's if (true == whatever).

Possible example...

std::map<int,bool> items;

...

if (items.at (key) == true)
{
  ...
}

Neither at nor items expresses that the value is boolean. if (item) doesn't read right - if anything, it suggests "if this is an item" which isn't the intent here. if (item == true) doesn't have this smell.

A smell is a distraction every time you revisit that code even if the smelly code is correct, so a couple of extra tokens is IMO worthwhile to clear that smell.

This is a reason why I'd generally prefer to name something is_whatever rather than whatever_flag. While whatever_flag is clearly intended to be a boolean, it still doesn't read quite right. It's the "grammar" of abbreviated English - "if flag" suggests "if this is a flag" rather than "if this flag is set".

Obviously, some newbies write if (whatever == true) because they have this mental template where every if needs a relative operator. This template is wrong, so this is one of those idiot-newbie stereotypes. There are some other similar cases, mostly obvious, like writing value + 0, value * 1 or value && true. A slightly surprising one from Haskell...

main = do putStrLn "Hello World"

In this, the "mental template" is that any sequence of monadic actions requires a do. In this case, though, there's only one monadic action so there's no need to compose a sequence of actions into a single action. All that's needed is...

main = putStrLn "Hello World"

In this case, I'll reserve judgement on whether it's occasionally worth keeping the do for readability - I don't have the experience.

Anyway, personally, I think that having ridiculed people for doing something, it's hard to accept that sometimes there's a valid case for doing it. After all, the mental template is that if (whatever == true) is automatically ridiculous.

Certainly if you want a quiet life, it's best to never do that.

1

There is no advantage to doing it that way. It could make it slightly more readable if it's not obvious from the function name that it should return a boolean, but in that case you'd be better off renaming the function

Matthew Mcveigh
  • 5,695
  • 22
  • 22
1

There is no advantage and the compiler will probably throw the "true =="-part away. If the function name makes clear that it returns a boolean (e.g. starting with "is" or "has"), then you can immediately see that it returns a boolean.

Maybe the code used to return error-codes as return values and this was refactored to a boolean, but the return-value check was not removed?

HuHu
  • 11
  • 2
  • 1
    In which case, it wouldn't be `true == ...`, but `someValue == ...`. (Of course, the idea of putting the constant on the left side of the `==` is rather twisted to begin with.) – James Kanze Aug 29 '13 at 11:04
  • Thanks for that :) ... no, in this case the code was always meant to be boolean (or was edited so that it was obviously meant to be) – code_fodder Aug 29 '13 at 11:14
  • @JamesKanze Some people advocate constant on the left in order to protect against the dreaded "assignment instead of comparison" typo (i.e. to prevent `if (3 = a) ...` from compiling.) Not that it makes sense in this context. – Rafał Dowgird Aug 29 '13 at 12:02
  • 1
    @Rafał Dowgird - that's called "Yoda conditions". The kind of error it's there to prevent (`if (a = 3)` rather than `if (a == 3)`) should trigger a warning in modern compilers anyway. –  Aug 29 '13 at 12:19
1

There is no difference except the code with explicit comparison is longer and more difficult to read.