Do you not understand booleans?
-
Believe it or not, but my high school teacher recommended doing this "because it's more clear". I was the only one ignoring that and only got flak for it.
Jeroen De Dauw (blog | Twitter | Identi.ca)
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}Which just show that there are 10 types of people in the world ...
-
CDP1802 wrote:
why not if (flag == true)?
Indeed, I prefer that when writing in C. One thing that drives me nuts with C is reading things like:
char* s = ...
if ( s ) ...
:mad:
Don't see why. The C language explicitly defines that comparison to work for null vs. non-null values, with the expected behaviour.
-
Fine. Now what if a is a (signed) integer and has a negative value? Or what if a is a pointer which is currently NULL? Without having defined any value for TRUE or FALSE and without knowing how NULL was defined somewhere deep in the libraries, how do you now know which code will be executed and which not? Even if NULL is usually defined as 0x00, you cannot expect this to be true for every compiler. And what can happen if you use another compiler?
int* a = NULL;
int b = -42;if(a)
{
// We should not need to know how NULL is defined and therefore can never know wether
// or not this code block will ever be entered
}if(a == NULL)
{
// Now we explicitly compared with NULL and it is clear when this code will be executed
}if(b)
{
// Negative values are undefined and it is up to the compiler wether a negative value is
// seen as 'true' or as 'false'
}if(b < 0)
{
// Explicitly testing the variable again removes all uncertainties
}And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse.Simple, where the value isn't boolean or a pointer, don't use that technique to test for 0. In C, I always used "if (c == 0)" for integer values, "if (c)" for booleans. Its the only safe way to do it, as there's no safe value for "true" for an integer. e.g. in bitwise arithmetic, commonly a bitfield may be tested for one or more of a number of bits set using a mask. The result may be 0 (none set) or some non-zero value (where 1 or more is set). In this case, the C behaviour works as expected, and indeed it was designed for exactly this kind of usage.
-
Simple, where the value isn't boolean or a pointer, don't use that technique to test for 0. In C, I always used "if (c == 0)" for integer values, "if (c)" for booleans. Its the only safe way to do it, as there's no safe value for "true" for an integer. e.g. in bitwise arithmetic, commonly a bitfield may be tested for one or more of a number of bits set using a mask. The result may be 0 (none set) or some non-zero value (where 1 or more is set). In this case, the C behaviour works as expected, and indeed it was designed for exactly this kind of usage.
Rob Grainger wrote:
e.g. in bitwise arithmetic, commonly a bitfield may be tested for one or more of a number of bits set using a mask. The result may be 0 (none set) or some non-zero value (where 1 or more is set). In this case, the C behaviour works as expected, and indeed it was designed for exactly this kind of usage.
Works as expected? What if 'one or more flag set' is still too weak a condition? What if you need to know wether or not all flags in the mask are set? We really have three cases here and using the C behavior here may actually be suboptimal:
LONG lResult = lFlagWord & lMask;
if(lResult == 0)
{
// no flags in the mask are set
}
else if(lResult == lMask)
{
// all flags in the mask are set
}
else
{
// only some of the flags in the mask are set
}And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse. -
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}I know that 0 is false (or FALSE) and anything else is true (or TRUE) disappointed to read through the comments ;P ... old C++ days ... since C++ defines "true" and "false" because defines the "bool" type the trick comes around when the main function must return 0 if everything is OK :omg:
--------- Antonio
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}> if (isUDPSetup()==true) You won't believe, but stupid hindu who wrote WPF (and .NET probably) follow logic like this! Look at ShowDialog() function - at WinForms time it was simple 'bool ShowDialog()' and code looked perfect: 'if (ShowDialog()) blah..'. Now they introduce... "bool?"!!! And code MUST look like "if (ShowDialog() == true) then...". You see that?! Really stupid....
-
You are a lucky man if that's all that makes your life miserable. Some of this is awkward, especially the second example, but where exactly is the horrible part? What damage is done? Does it not work or are there possible side effects? Does it lead to worse code being generated by the compiler? I think the first two just show a clumsy coding style, but no real harm comes from it. Except, of course, curling up your toenails when you have to read it. Usually I do the last one myself. It is at least the same way you compare any other variable (if (x == 0), so why not if (flag == true)?) and I prefer to make it absolutely clear what I want to accomplish. The compiled code remains absolutely the same, no matter how you write it. That's why I see the last one more as a matter of preferences, but not as anything important.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse.CDP1802 wrote:
t is at least the same way you compare any other variable (if (x == 0), so why not if (flag == true)?
My reasoning is you could take any boolean expression, and instead of writing
if (X)
you could write
if (X==true)
to make it really clear. But you could then also write
if ((X==true)==true)
to make it really really clear. So I'm thinking let's drop all implied trailing "==true"'s in a mental tail call optimization. ;)
-
CDP1802 wrote:
t is at least the same way you compare any other variable (if (x == 0), so why not if (flag == true)?
My reasoning is you could take any boolean expression, and instead of writing
if (X)
you could write
if (X==true)
to make it really clear. But you could then also write
if ((X==true)==true)
to make it really really clear. So I'm thinking let's drop all implied trailing "==true"'s in a mental tail call optimization. ;)
And I think your reasoning would be wrong. It IS more clear to write if (X==true). Just because you don't like it does not mean it is not more clear, especially to junior programmers. I am the senior lead and I instruct ALL of our programmers under me to write if (X==true). It doesn't cost the compiler anything and it makes it understandable by even the junior most person quickly. It is all about proper maintenance and thinking about the coder behind you instead of just yourself. X is a variable so comparing it like another variable is both consistent and readable.
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}Wouldn't this be more run-time efficient, as the value of
updatecan be cached in a register ?void setNeedsUpdate(bool update)
{
if (update == true)
{
NeedsUpdate= update;
}
else
{
NeedsUpdate= update;
}
};->
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
} -
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}I hate this code too. Maybe in C you can do this but in C# it is a travesty. What I do is create self-documenting booleans so that you write code like this:
if (DoorIsOpen)
{
// Do Something
}
else
{
// Do Something else
}I feel that this eliminates the need for any of the crap above. It is true that the compiler doesn't care but just remember the the point of code is so humans can understand it. If we only cared about the computer understanding it, we would all be typing 1s and 0s. Write code so that those who come after you can read it without being distracted by how horrible it is.
"I don't believe it." "That is why you fail." -- Empire Strikes Back Shameless blog plug - www.geekswithblogs.net/jboyer
-
I know that 0 is false (or FALSE) and anything else is true (or TRUE) disappointed to read through the comments ;P ... old C++ days ... since C++ defines "true" and "false" because defines the "bool" type the trick comes around when the main function must return 0 if everything is OK :omg:
--------- Antonio
PinballWizard wrote:
the main function must return 0 if everything is OK
That's why zero should mean true. :sigh:
-
And yet that style of testing for non-null is the standard in, for example, JavaScript. (Personally I have no problem with it in C either, for pointer or unsigned types ... with signed values it's bad because as pointed out someone could potentially switch negative values' interpretation out from under you.) When comparing against something which is (semantically or definitively, depending on your language) a boolean, though, there's absolutely no reason to make the reader parse more text before he can understand your statement. Adding the ' == true' provides zero clarification value, as anyone who vaguely knows the language knows that if tests check for a true value. In your example here, adding the ' != NULL' does provide a little clarification, so it's pretty much a value-neutral stylistic choice to add it or not. (Someone might suggest that NULL could evaluate to true, but anyone who makes that #define is far more culpable for any confusion than the person who writes the check you put here, and in any professional context that won't be the case.)
BobJanova wrote:
Adding the ' == true' provides zero clarification value
Exactly! Not only that, it also adds the potential for a catastrophic typo:
=trueinstead of==true. I'd even go so far as to include comparisons to user-defined symbols that represent boolean states, such asTRUEandFALSE: ifif(x)is not equivalent toif(x==TRUE), then the definitions are wrong, since they do not adhere to the rules of boolean logic! In that case not the comparison needs to be 'fixed', but the definitions! -
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}Let's see... The first example I don't like - not for the if statement, but for the premature returns! Personally, I'd make it a one-liner, but I don't consider it all that horrible apart from the returns. The second ... well you could argue it's slightly more readable for inexperienced programmers, but if that is your purpose then you should write an
if/elsestatement, not use?:. The next is a double horror for comparing to boolean constants and unnecessary nesting, but I'd forgive the nesting for the off-chance of later maintenance introducing additional statements that depend on only the first condition. The last ... I suppose if you need to be able to run your code through really old C compilers that don't have their own built-in definitions ofbool,trueandfalse, then it makes sense not to assign one boolean variable blindly to another, but instead make anif/elsestatement to catch the case where the argument is neithertruenorfalse. But even then,==trueis horrible. So, in short, all of the examples are indeed bad style, IMHO, but I agree with the others that no harm is done, except maybe a minor hit in maintenance effort for posting it in the Hall of Shame. ;) None of this will cause inefficiency either, since compilers are smart enough to produce efficient code even from that kind of code. -
The horrors I've seen:
void setVisible(bool isVisible)
{
if(isVisible.ToString().ToLower() == "true")
{
this.Visible = true;
}if(isVisible.ToString().ToLower() == "false")
{
this.Visible = false;
}
}:wtf: You're making this up, aren't you? :omg:
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}They were probable taught to program by some CS grad student who'd never done much significant real-world coding. That's right along the lines of the kinds of stupidity my teachers would teach us when I was an undergrad. Like everybody else, I picked up the stupidity too.. which lasted until I saw the other way and had to debug code to a schedule that was broken by such nonsense. I still do the compare to NULL sometimes, but I believe the C standard now defines NULL pointers as a false boolean value, so it is redundant and I'm trying to retrain away from it. Besides, boost smart pointers, which we use a lot in our code, have an override to generate a bool result for just such kinds of pointer checks and make comparing the raw pointer to NULL harder.
We can program with only 1's, but if all you've got are zeros, you've got nothing.
-
:wtf: You're making this up, aren't you? :omg:
-
This stuff drives me up the wall!!!
bool is_queue_empty(void)
{
if (queue_length==0)
{
return true;
}
else
{
return false;
}
}Or this:
bool counter_zero = counter==0 ? true : false;
Or this:
if (isUDPSetup()==true)
{
if ((forceSend==false))
{
...
}
}(Variable names have been changed to protect the guilty) Or this *New one*:
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
} -
And it obviously makes the reader scared of what else might come from that code who's developer can't understand even a booleans...
"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson