true/false statements in if clause
-
I mostly disagree with the others. 1. You need better identifier names. Then the short form is the right one:
if (printingToFile) printToFile();
2. it does not make sense comparing with true explicitly, as can be proven ex absurdo: If you think
if (a==true) ...
is any good, then the following is even better:
if ((a==true)==true) ...
3. when the variable's type is NOT boolean and you need an explicit constant value, then one can argue putting the constant first is safer, as it causes an error when accidentally = is used instead of ==. However, a decent compiler will generate a warning when you accidentally write:
if (myInteger=1) ...
:)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles] Nil Volentibus Arduum
Please use <PRE> tags for code snippets, they preserve indentation, and improve readability.
I don't agree with your point 2 proof (though it is clever). :)
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles] -
Why is more apropriate to write
if(true == a)
instead of
if(a == true)
and why the
if(a)
is avoidable. I mean not avoidable, but the first two are preferable.
Generally prefer the third form - if someone needs reminding that a logical expression resolves to
true(non-zero in older languages) orfalse(0 in older languages) in an algol derived language then they need a refresher. If you use decent names, know boolean algebra and wrap up complicated logical expressions in functions you can't go wrong. Or rather you probably won't go wrong, and if you did then direct comparison withtrueorfalse(either way around) isn't going to help you. Cheers, Ash -
i personally dislike
if (a)
because it isn't obvious at the very first look what exactly 'a' is, it could be a bool, an int, a pointer, an instance of a class with a bool operator, and it can be confusing if you are e.g. reading someone else's code and do not have an in-depth knowledge of what is what and why.
> The problem with computers is that they do what you tell them to do and not what you want them to do. < > "It doesn't work, fix it" does not qualify as a bug report. < > Amazing what new features none of the programmers working on the project ever heard of you can learn about when reading what the marketing guys wrote about it. <
Actually if you use this form strictly to check if a is true instead of for example checking if the integer value is 0 this form is very clear.
-
Actually if you use this form strictly to check if a is true instead of for example checking if the integer value is 0 this form is very clear.
As Richard said, it is a matter of style really. That form can also be very easily readable with well named variables, e.g:
if (noMoreItemsLeft) EndOfItems();
looks quite obvious when you read through the code, but
if (!ptrToNextItem) EndOfItems();
you have to 'analyze' this, if not ptrToNextItem, this means, if ptrToNextItem is zero, then... I also don't like situations like this:
class AClass
{
public:
int Integer;
bool Bool;AClass(int i, bool b):Integer(i),Bool(b) {}
operator bool() const { return Bool; }
AClass &operator =(int i) { Integer = i; return *this; }
};
...
AClass A(0, true);
if (A) A = 3; //So here A's bool operator will be used and since it is true, A = 3 will run, however, A's
//integer value is 0, if we were to test for that, A = 3 wouldn't run. Ok, in this example
//it is quite obvious but imagine a big class with loads of operators and methods and this
//ifembedded in a "busy" part of the code, "far far" away from the
//declaration of the class. If you were to "scan" through the code and see this you could
//think: 'what this does is making A's value 3 when A isn't 0, A is likely to be an
//integer'...
...> The problem with computers is that they do what you tell them to do and not what you want them to do. < > "It doesn't work, fix it" does not qualify as a bug report. < > Amazing what new features none of the programmers working on the project ever heard of you can learn about when reading what the marketing guys wrote about it. <
-
As Richard said, it is a matter of style really. That form can also be very easily readable with well named variables, e.g:
if (noMoreItemsLeft) EndOfItems();
looks quite obvious when you read through the code, but
if (!ptrToNextItem) EndOfItems();
you have to 'analyze' this, if not ptrToNextItem, this means, if ptrToNextItem is zero, then... I also don't like situations like this:
class AClass
{
public:
int Integer;
bool Bool;AClass(int i, bool b):Integer(i),Bool(b) {}
operator bool() const { return Bool; }
AClass &operator =(int i) { Integer = i; return *this; }
};
...
AClass A(0, true);
if (A) A = 3; //So here A's bool operator will be used and since it is true, A = 3 will run, however, A's
//integer value is 0, if we were to test for that, A = 3 wouldn't run. Ok, in this example
//it is quite obvious but imagine a big class with loads of operators and methods and this
//ifembedded in a "busy" part of the code, "far far" away from the
//declaration of the class. If you were to "scan" through the code and see this you could
//think: 'what this does is making A's value 3 when A isn't 0, A is likely to be an
//integer'...
...> The problem with computers is that they do what you tell them to do and not what you want them to do. < > "It doesn't work, fix it" does not qualify as a bug report. < > Amazing what new features none of the programmers working on the project ever heard of you can learn about when reading what the marketing guys wrote about it. <
-
I'd say that was more a criticism of the inappropriate use of operator overloading rather than a problem with not comparing the results of logical expressions to true and false. Cheers, Ash
I have seen this being done (not by me). Anyways, am just trying to say that "in my oppinion" simply writing
if (whatever) doit();isn't always completely clear and obvious. But i am repeating myself :) . This is indeed a question of style and personal oppinion, i am not saying it is good or bad, just sharing my point of view.> The problem with computers is that they do what you tell them to do and not what you want them to do. < > "It doesn't work, fix it" does not qualify as a bug report. < > Amazing what new features none of the programmers working on the project ever heard of you can learn about when reading what the marketing guys wrote about it. <
-
I mostly disagree with the others. 1. You need better identifier names. Then the short form is the right one:
if (printingToFile) printToFile();
2. it does not make sense comparing with true explicitly, as can be proven ex absurdo: If you think
if (a==true) ...
is any good, then the following is even better:
if ((a==true)==true) ...
3. when the variable's type is NOT boolean and you need an explicit constant value, then one can argue putting the constant first is safer, as it causes an error when accidentally = is used instead of ==. However, a decent compiler will generate a warning when you accidentally write:
if (myInteger=1) ...
:)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles] Nil Volentibus Arduum
Please use <PRE> tags for code snippets, they preserve indentation, and improve readability.
-
Agreed 100%. Remember these ones:
BOOL bDoIt = ::ExternFunction();
if (bDoIt == TRUE)where ExternFunction() according to documentation returns non-zero on success? It's not a matter of style. It's a matter of stupidity.
Niklas Lindquist wrote:
BOOL bDoIt = ::ExternFunction(); if (bDoIt == TRUE)
Niklas Lindquist wrote:
It's a matter of stupidity.
May be I'm being a bit stupid but, what exactly is "stupid" in the code above?
if (bDoIt)would've looked nicer butif (bDoIt == TRUE)isn't bad either.
It's better to know some of the questions than all of the answers.
Pravin. -
Niklas Lindquist wrote:
BOOL bDoIt = ::ExternFunction(); if (bDoIt == TRUE)
Niklas Lindquist wrote:
It's a matter of stupidity.
May be I'm being a bit stupid but, what exactly is "stupid" in the code above?
if (bDoIt)would've looked nicer butif (bDoIt == TRUE)isn't bad either.
It's better to know some of the questions than all of the answers.
Pravin.It's ultra-bad. Returning non-zero on success, might just as well mean returning 2, 3 or, God forbid, even 4. Comparing it to TRUE (i.e. 1) might not give the anticipated result. The only thing you can compare it to is FALSE, if (bDoIt != FALSE), would have been the correct usage. But that really doesn't help anyone, does it. Good naming removes the need for such constructs. It's about readability. Consider
if (signalIsOn) {}
if (signalIsOn == true) {}The second construct is just plain silly, and actually reduces readability.
-
It's ultra-bad. Returning non-zero on success, might just as well mean returning 2, 3 or, God forbid, even 4. Comparing it to TRUE (i.e. 1) might not give the anticipated result. The only thing you can compare it to is FALSE, if (bDoIt != FALSE), would have been the correct usage. But that really doesn't help anyone, does it. Good naming removes the need for such constructs. It's about readability. Consider
if (signalIsOn) {}
if (signalIsOn == true) {}The second construct is just plain silly, and actually reduces readability.
OK, now I get it. To be honest, I had never thought about this side effect. But, to me what seems to be the real problem here is the use of
BOOL(which is essentiallyint)in place ofbool. Abool bDoItwould have responded to any non-zero number by simply turning itself totrue. Does that mean we should avoid usingBOOLin general?
It's better to know some of the questions than all of the answers.
Pravin. -
OK, now I get it. To be honest, I had never thought about this side effect. But, to me what seems to be the real problem here is the use of
BOOL(which is essentiallyint)in place ofbool. Abool bDoItwould have responded to any non-zero number by simply turning itself totrue. Does that mean we should avoid usingBOOLin general?
It's better to know some of the questions than all of the answers.
Pravin.PravinSingh wrote:
the real problem here is the use of BOOL
I admit that this is slightly off topic, but still, I think it's worth mentioning. Avoiding BOOL in general is a good thing. But in reality, you would probably use a mix if you are hacking Windows API. The language evolves, but sadly, a lot of features are not used using excuses such as 'we need to handle an old code base' or 'we need to support an ancient compiler'. They might be valid excuses, but you could be stuck at that forever. Whenever possible, embrace what the technology gives. Also, embrace boolean algebra and proper naming :)