Do you not understand booleans?
-
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
-
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;
}Hey, you're coding in a language that I don't use, but I'm totally with you. This is one of my pet peeves, too. One of my languages is Visual FoxPro, which has the "Immediate IF" ternary function, IIF: IIF(expr, a, b) If expr is true, a is returned, otherwise b is returned. So what I see is people doing this: MyBoolVar = IIF(SomeVar = 2, .t., .f.) Which could be simplified to: MyBoolVar = SomeVar = 2 Much cleaner. Or they expand it into a regular IF statement similar to what you posted. When I see this it does worry me. If programming should result from logical thinking and a programmer has trouble understanding logical values (boolean values), then should we wonder about the rest of their skills?
-
Wow - that link is worthy of a top position in the Hall of Shame all by itself! :omg:
-
Yes, I suppose this simply comes from the old C++ days where there was no boolean type. To avoid side effects or compiler specific behavior, I always explicitly specified what I was testing. But I would write it like this:
if((flag1 == true) &&
(flag2 == true) &&
(flag3 == true))
{
...
}Usually it does not look so uniform. If I really had so many different flags, I would think about using a flag word and testing all flags in one go.
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.(edit: I posted this as a reply to a message that is now now longer there, oh well) I do not write out the boolean, but I do write each condition on a new line. In addition to that, I start the new line with the operator, this way, it is very easy to see at the beginning of the line that it is a continuation of the previous line, and what the operation is:
if( flag1
&& (someOtherFlagThatWillSqrewWithTheLayout == MagicNumbers.Ten)
&& (flag3 == somethingElseCompletely))
{
...
}I use this style with anything that will make a line of code too long:
//Contrived Deep Nesting, line too long
var firstChildRow= SomeTypedDataSetWithSillyLongNameThatFillsTheEntireCodeWindow.Tables[0].ChildRelations[0].ChildTable.Rows[0];
//Broken up for readability. Note that I start with the 'dot'.
var firstChildRow= SomeTypedDataSetWithSillyLongNameThatFillsTheEntireCodeWindow
.Tables[0]
.ChildRelations[0]
.ChildTable
.Rows[0]; -
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;
}void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}This would be better if they returned it too. Nothing like getting back what you put into it.
bool setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
return NeedsUpdate;
} -
void setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
}This would be better if they returned it too. Nothing like getting back what you put into it.
bool setNeedsUpdate(bool update)
{
if ((update==true))
NeedsUpdate=true;
else
NeedsUpdate=false;
return NeedsUpdate;
}The second is better than the first. At least if you're going to set the value in this ridiculous fashion, make sure that it worked. :)
I wasn't, now I am, then I won't be anymore.
-
Yes, I suppose this simply comes from the old C++ days where there was no boolean type. To avoid side effects or compiler specific behavior, I always explicitly specified what I was testing. But I would write it like this:
if((flag1 == true) &&
(flag2 == true) &&
(flag3 == true))
{
...
}Usually it does not look so uniform. If I really had so many different flags, I would think about using a flag word and testing all flags in one go.
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.I came from the same dark ages and indeed untill some time ago i also had the tendency to check the boolean value. Particularly the BOOL was a nasty one as you could (with good sense) only check that to FALSE. One other 'trick' i got used to apply was swapping the variable and the value E.g.
if(FALSE != flag)
{Logically this seems a bit odd but then again it did protect me against typo's like:
if( flag = FALSE)
{Today this will generate a compiler warning but that has not always been the case and if you have a special vendor type compiler; you may still face the same. Why check on FALSE? Simple; that was defined (as 0), any one could set the BOOL to TRUE, 1, 2 etc. Don't you love the compilers of today? Or better yet, those of tomorrow? Cheers, AT
Cogito ergo sum
-
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:
-
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 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 is, at best, a matter of opinion. For me it is less clear to write if(x == true) because I have to read twice as much text to get the meaning &dnash; just as it's unclear to give a method a 300 character name. I have never met anyone who is confused by if(booleanVariable) and if they are then they shouldn't be programming until they learn the language they're using – if they have trouble reading that then do you really want them poking around your pointer code, or reflection in C#, or constructor injection frameworks, or any of the other million things any real world app has that are far more confusing? Readability is all about having a single, clear, unambiguous meaning for a statement as quickly as possible. if(x) and if(!x) are short, clear and obviously different from each other (as long as you're using a font where ! is more than 2 pixels wide, heh). if(x == true) adds nothing, is easier to mix up with closely related but different statements (if(x = true) or if(x == True) or if(x == "true") etc) and doesn't immediately show that x is a boolean or castable to one until you read the whole thing. You are on your way to becoming one of the micromanaging senior leads who appear on The Daily WTF issuing that kind of order based on your personal opinion of readability.
-
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.Hey, I went and looked this up, the standard states explicitly that "In both forms, the first substatement is executed if the expression compares unequal to 0." http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf[^] page 147 That is, negative numbers are defined by standard to be true for conditional expressions.
-
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
i might be showing my age here but there used to be a readability test known as "the telephone test" (from Kerningham and Plauger) - Read your code to someone over the phone. If they can't understand it, try writing the code again. admittedly this has its problems but one of the upshots was that you should just name your booleans for the thing they test and then it reads well. also, comparing to "== true" or "== false" obviously breaks this readability test :)
-
i might be showing my age here but there used to be a readability test known as "the telephone test" (from Kerningham and Plauger) - Read your code to someone over the phone. If they can't understand it, try writing the code again. admittedly this has its problems but one of the upshots was that you should just name your booleans for the thing they test and then it reads well. also, comparing to "== true" or "== false" obviously breaks this readability test :)
mostlyharmless1964 wrote:
i might be showing my age here
Wow, never heard of it. Was it the time when the compilers were birds inside stone cases? :laugh:
"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson
-
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 fully agree. This kind of stuff makes the code SO hard to read. And so many people do this too - just look at all the examples you were able to find! Atrocious. When will people learn to put spaces around their operators !?!?! :cool:
Clive Pottinger Victoria, BC
-
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.
Why do boolean variables exist? To store and retrieve boolean expressions (TRUTH values). They were invented SO THAT we can write code like
if (X)
otherwise we could just as well remove the boolean type and work with integer flags like
if (X==1)
This was one of the issues people had with C. No proper boolean type. But now we have a proper boolean type so don't reduce it to a "flag value" that needs to be compared to something to find the truth. It holds the truth all on its own. That's its job.