Variation of bad boolean check.
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
Pete O'Hanlon wrote:
variable names changed to protect the innocent
What innocent?
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
Without knowing the 'typeness' of instance.Variable, what makes this code particularly shameful? :confused:
Chris Meech I am Canadian. [heard in a local bar] I agree with you that my argument is useless. [Red Stateler] Hey, I am part of a special bread, we are called smart people [Captain See Sharp] The zen of the soapbox is hard to attain...[Jörgen Sigvardsson] I wish I could remember what it was like to only have a short term memory.[David Kentley]
-
Without knowing the 'typeness' of instance.Variable, what makes this code particularly shameful? :confused:
Chris Meech I am Canadian. [heard in a local bar] I agree with you that my argument is useless. [Red Stateler] Hey, I am part of a special bread, we are called smart people [Captain See Sharp] The zen of the soapbox is hard to attain...[Jörgen Sigvardsson] I wish I could remember what it was like to only have a short term memory.[David Kentley]
What happens if you simply leave out the "? true : false" part? ... .. . got it? :)
Regards, mav -- Black holes are the places where God divided by 0...
-
What happens if you simply leave out the "? true : false" part? ... .. . got it? :)
Regards, mav -- Black holes are the places where God divided by 0...
OK - that sig alone gets my 5. It actually made me laugh.
Deja View - the feeling that you've seen this post before.
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
Well, this code is 100% correct imo (under some conditions). If Variable is an Integer, the Visual Studio C++ compiler shows a warning (C4800, type' : forcing value to bool 'true' or 'false' (performance warning)) when you use the following code:
int i = 1; bool c = i;
So, I guess the guy only wanted to get rid of the compiler warning :) -
Well, this code is 100% correct imo (under some conditions). If Variable is an Integer, the Visual Studio C++ compiler shows a warning (C4800, type' : forcing value to bool 'true' or 'false' (performance warning)) when you use the following code:
int i = 1; bool c = i;
So, I guess the guy only wanted to get rid of the compiler warning :)While it may be correct, it could also have been written:
bool isValue = (instance.Variable == 1);
. The true/false at the end is redundant.
Deja View - the feeling that you've seen this post before.
-
While it may be correct, it could also have been written:
bool isValue = (instance.Variable == 1);
. The true/false at the end is redundant.
Deja View - the feeling that you've seen this post before.
-
While it may be correct, it could also have been written:
bool isValue = (instance.Variable == 1);
. The true/false at the end is redundant.
Deja View - the feeling that you've seen this post before.
Pete O`Hanlon wrote:
. The true/false at the end is redundant.
But hardly "Hall of Shame" worthy.
"Approved Workmen Are Not Ashamed" - 2 Timothy 2:15
"Judge not by the eye but by the heart." - Native American Proverb
-
Pete O`Hanlon wrote:
. The true/false at the end is redundant.
But hardly "Hall of Shame" worthy.
"Approved Workmen Are Not Ashamed" - 2 Timothy 2:15
"Judge not by the eye but by the heart." - Native American Proverb
It is when there are about 30 of these tests in the same routine.
Deja View - the feeling that you've seen this post before.
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
Pete O`Hanlon wrote:
bool isValue = (instance.Variable == 1 ? true : false);
AFAIK, there is just one single value (for a 'bool') that's really defined, and that is the value for false (0). True is defined as 'not false', so it can be any value, except for 0. In Win32, there are cases where 'true' can be either 1 or -1. An example: TRUE is 1 VARIANT_TRUE is -1 So IMO, the code would be useful if it looked like this:
bool isValue = (instance.Variable == 0 ? false : true);
Alcohol. The cause of, and the solution to, all of life's problems - Homer Simpson
-
Pete O`Hanlon wrote:
bool isValue = (instance.Variable == 1 ? true : false);
AFAIK, there is just one single value (for a 'bool') that's really defined, and that is the value for false (0). True is defined as 'not false', so it can be any value, except for 0. In Win32, there are cases where 'true' can be either 1 or -1. An example: TRUE is 1 VARIANT_TRUE is -1 So IMO, the code would be useful if it looked like this:
bool isValue = (instance.Variable == 0 ? false : true);
Alcohol. The cause of, and the solution to, all of life's problems - Homer Simpson
kakan wrote:
bool isValue = (instance.Variable == 0 ? false : true);
Better still:
bool isValue = (instance.Variable != 0);
To me this is clearer.
Failure is not an option - it's built right in.
-
kakan wrote:
bool isValue = (instance.Variable == 0 ? false : true);
Better still:
bool isValue = (instance.Variable != 0);
To me this is clearer.
Failure is not an option - it's built right in.
I would do it the same way you do. I had two reasons for responding the way I did: 1. I wanted the smallest possible alteration to the original code. 2. In the past, I have given "smartcoded" examples and has been flamed for it, with comments like "unredable" and "impossible to understand". (My response that the compiler could handle my source code yielded heaps of "1" votes.) So now I try to keep my code examples as easy to understand as possible.
Alcohol. The cause of, and the solution to, all of life's problems - Homer Simpson
-
I would do it the same way you do. I had two reasons for responding the way I did: 1. I wanted the smallest possible alteration to the original code. 2. In the past, I have given "smartcoded" examples and has been flamed for it, with comments like "unredable" and "impossible to understand". (My response that the compiler could handle my source code yielded heaps of "1" votes.) So now I try to keep my code examples as easy to understand as possible.
Alcohol. The cause of, and the solution to, all of life's problems - Homer Simpson
I actually posted this because I didn't see the need for the true/false test. The result of the equality test is a boolean condition, so either the item is equal to 1 (true) or it isn't (false). It just seemed that the coder had put in unnecessary code because he thought it was clever. What made it worse, was that there were about 30 of these tests in one routine. Argggh.
Deja View - the feeling that you've seen this post before.
-
I actually posted this because I didn't see the need for the true/false test. The result of the equality test is a boolean condition, so either the item is equal to 1 (true) or it isn't (false). It just seemed that the coder had put in unnecessary code because he thought it was clever. What made it worse, was that there were about 30 of these tests in one routine. Argggh.
Deja View - the feeling that you've seen this post before.
Yes, under normal circumstances, it's a total waste, I agree. The reason why I responded in the first place was that I once spent quite some time to figure out why a function (that turned out to want a VARIANT_BOOL) didn't react when I sent a (BOOL) TRUE to it. Then I learned the difference between (BOOL) TRUE (1) and (VARIANT_BOOL) TRUE (-1). And after that, I always test for false or not false. It can't go wrong. All versions of false *is* 0 and nothing else.
Alcohol. The cause of, and the solution to, all of life's problems - Homer Simpson
-
OK - I'm just QAing some code, and I've come across the following (variable names changed to protect the innocent):
bool isValue = (instance.Variable == 1 ? true : false);
<Ironic>I can only assume that this is to protect from that other non-nullable boolean condition of "maybe(ish)".</Ironic>
Deja View - the feeling that you've seen this post before.
Well, may be it's not the best way, but there could be good resons: 1)
instance.Variable
is a generic class you don't know the type, but you kow it supportsbool operator==(int)
. Assuming that an implicit conversion tobool
will do the same is not obvious. (And such a conversion may be even not existent) 2) ifinstance.Variable
is anint
, assigning anint
to abool
gives a warning. That code eliminates it 2a) But there is a int-to-bool native converter: the!!
pseudo-opreratorbool isValue(!!instance.Variable);
2 bugs found. > recompile ... 65534 bugs found. :doh:
-
Well, may be it's not the best way, but there could be good resons: 1)
instance.Variable
is a generic class you don't know the type, but you kow it supportsbool operator==(int)
. Assuming that an implicit conversion tobool
will do the same is not obvious. (And such a conversion may be even not existent) 2) ifinstance.Variable
is anint
, assigning anint
to abool
gives a warning. That code eliminates it 2a) But there is a int-to-bool native converter: the!!
pseudo-opreratorbool isValue(!!instance.Variable);
2 bugs found. > recompile ... 65534 bugs found. :doh:
OK. Perhaps I'm being a bit too subtle here.
bool isVariable = (instance.Variable == 1);
That's it. That's all you need to do. The check instance.Variable == 1 will only return true or false so there's no need to the ? true : false check. They are redundant. As I've stated before - this method did about 30 of these checks. The comparison is redundant in each case.
Deja View - the feeling that you've seen this post before.
-
Pete O`Hanlon wrote:
. The true/false at the end is redundant.
But hardly "Hall of Shame" worthy.
"Approved Workmen Are Not Ashamed" - 2 Timothy 2:15
"Judge not by the eye but by the heart." - Native American Proverb
I have to disagree. Amongst beginners tertiary operators are notoriously hard to read. Anybody who adds them superflously does deserve the hall of shame.