|
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?
|
|
|
|
|
Without knowing the 'typeness' of instance.Variable, what makes this code particularly shameful? 
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...
|
|
|
|
|
OK - that sig alone gets my 5. It actually made me laugh.
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;<br />
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.
|
|
|
|
|
Ah, now I understand, thx
Shame on me 
|
|
|
|
|
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.
|
|
|
|
|
I have to disagree. Amongst beginners tertiary operators are notoriously hard to read. Anybody who adds them superflously does deserve the hall of shame.
|
|
|
|
|
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.
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
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 supports bool operator==(int). Assuming that an implicit conversion to bool will do the same is not obvious. (And such a conversion may be even not existent)
2) if instance.Variable is an int, assigning an int to a bool gives a warning. That code eliminates it
2a) But there is a int-to-bool native converter: the !! pseudo-oprerator
bool isValue(!!instance.Variable);
2 bugs found.
> recompile ...
65534 bugs found.
|
|
|
|
|
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.
|
|
|
|
|
I found this piece of code on the server side of an ASP.NET C# application. I think he was waiting for the text to change at the browser side before continuing. It didn't take me long to figure out why the page kept locking up.
while (Textbox3.Text == "")
{
}
|
|
|
|
|
varnk wrote: while (Textbox3.Text == "")
{
}
What a shame ....!!!! 
|
|
|
|
|
This one gave me a good laugh:
if (index == 1 || 2)
{
...bla
}
The guy was probably on the phone while writing it.
Company policy : no access to the internet but CP
~RaGE()
|
|
|
|
|
Another reason to like C#
Perhaps also another argument for the fans of
if ( 1 == index ) { ... }
|
|
|
|
|
When I was a noob programmer I would do things like that.
if (index == 1 || index == 2)
...
█▒▒▒▒▒██▒█▒██
█▒█████▒▒▒▒▒█
█▒██████▒█▒██
█▒█████▒▒▒▒▒█
█▒▒▒▒▒██▒█▒██
|
|
|
|
|
I think that's easier to read, and less ambiguous than this since you don't have to wonder if the equals was intended or a strict less than was actually wanted.
if ((1 <= index) && (index <= 2))
--
Rules of thumb should not be taken for the whole hand.
|
|
|
|
|
am i losing it?
what's wrong with that?
Russell
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
