|
From the ANSI C99 spec I have:
"
6.5.13-4
Unlike the bitwise & operator, the && operator guarantees left-to-right evaluation; there is a sequence point after the evaluation of the first operand. If the first operand compares equal to 0, the second operand is not evaluated.
"
Even Ritchie's C manual from 1974 says it's left-to-right. (It appears that B does not have these operators.)
In the K&R book, they're talking about a[i]=i++, not the && operator, plus that's a very old book.
You may certainly go right ahead and write code any way you like... I do.
|
|
|
|
|
Robert.C.Cartaino wrote: I think C++ has an "undefined order of evaluation".
C and C++ are both strict left to right evaluation order for logical operators, and always use short-circuiting. It allows constructs such as
void* p = GetPointer();
if (p != NULL && p->SomeMethod()
....
p->SomeMethod() will only be called if p is not NULL
|
|
|
|
|
see this[^]
"
&& Operator (C# Reference)
The conditional-AND operator (&& ) performs a logical-AND of its bool operands, but only evaluates its second operand if necessary.
"
"
x && y
if x is false, y is not evaluated (because the result of the AND operation is false no matter what the value of y may be). This is known as "short-circuit" evaluation.
"
Any language that doesn't do it that way is not a "real" programming language.
|
|
|
|
|
Robert.C.Cartaino wrote: Writing code that depends on order of evaluation is bad programming practice
If order of evaluation is not defined by the specification of the language, which is not true for logical operators in C/C++.
According to your statement one should not write something like this:
if( node && node->value )
node->value->Do();
Instead you need to have another if for this simple operation even if the previous example is perfectly legal and clean and the desired behavior is guaranteed by the language specification. Well I guess that the only bad programming practice here is that someone introduces unneeded code because he doesn't trust/know language.
|
|
|
|
|
Indeed. And should we question the order/precedence of the other operators as well?
Can we trust 3 + 4 * 5 to return 23? or might it return 35 once in a while?
I'm all for adding parentheses for clarity, but I don't get that confused with necessity.
|
|
|
|
|
You are depending on the order of evaluation of your if-statement for your logic to work. Will the PromptUser() operand be evaluated before the entire set of && operations are performed? I think C++ has an "undefined order of evaluation". Who knows. It should be irrelevant. Writing code that depends on order of evaluation is bad programming practice, regardless of language.
Sorry, but not knowing how your programming language works is bad practice (operator priority is very important in every language!). And depending on the order of your evaluations is the usual case in real world programming-logic. (I think you are mixing up here with the | and & operators and something a teacher told you once...).
I think C++ has an "undefined order of evaluation". Who knows
No - and I!!! 
|
|
|
|
|
The second certainly looks cleaner; but sometimes you need to do cleanup (dispose objects, free memory, release handles, etc.) before exiting a function / method. If you add code later that needs cleanup, and already coded the second way, you'll have a lot to change... in addition, code that is already out there running stable is more valuable than new code, since you risk introducing bugs by changing from your second example back to the first. So I'd definitely go for the first - good habits breed consistency.
|
|
|
|
|
I've got to say, i don't agree with that rule.
I will aim to return out the function either very early, or at the end. Trivial conditions should be treated as such, and I'd rather they returned at the top, than add a unnecessary level of nested bracers.
I have seen the result of blindly following that practice to it's conclusion - a mountain of if / else statements with embedded switches that was 12 deep in places. I would rather see:
<br />
<br />
if (guardCondition1)<br />
return guard1Default;<br />
<br />
if (guardCondition2)<br />
return guard2Default;<br />
<br />
if (guardCondition3)<br />
return guard3Default;<br />
<br />
<br />
<br />
<br />
return result;<br />
<br />
That said, throwing return statements in without any real eye on program flow can cause just as many problems.
Regards
Tris
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
Maybe WF fit such situation
The God created the world.
The programer made the world easy.
|
|
|
|
|
I completely agree with your sentiment. But in this specific example, it looks like processes generating FlagA, FlagB, and FlagC should always execute, regardless of whether any of the previous processes fail. Your guardConditionals don't allow for that.
|
|
|
|
|
Having been brought up on 'Structured Programming' I used to think that one-entry and one-exit point was the only way to write procedures.
Now I'm in favour of your approach - i.e. use guard conditions and exit early if need be. Oh and keep the procedure small, roughly no more than can fit on an A4 page.
One example of how not to do it that I came across was a C++ function that had around 500 lines of code with about 15 returns scattered throughout. It also had a couple of places where it could throw an exception!
David Rice
|
|
|
|
|
Having one return statement is still a worthwhile goal.
You just have to look at it both ways and decide which is "better" in each individual case.
I don't recall encountering any situations that required multiple return statements.
|
|
|
|
|
You can't make a rule like that because there is no one correct rule that works for every single function you're going to write in your life. Well, you can make the rule, but doing so will be counter-productive. Why create a state machine with local variables in a function (and don't forget to test every possible state!), just to avoid using a perfectly valid language feature?
|
|
|
|
|
it all depends on how much coffe i've had for the day...
do or do not, there is no try
|
|
|
|
|
Drinking coffee leads to good code. Drinking "energy drinks" leads to poor code.
|
|
|
|
|
Single exit , single entry . Its a mantra that leads to clarity
how about
if( !FlagA || !FlagB || !FlagC) // Select Variant on !Flagx or Flagx == false depending on language
DoOtherThing();
else
{
if(PromptUser())
DoSomething();
}
Not a horror though. My use of single entry and exit points in functions leads to more debates than anything. (My how the winter nights fly by)
|
|
|
|
|
Andrew Torrance wrote: if( !FlagA || !FlagB || !FlagC) // Select Variant on !Flagx or Flagx == false depending on language
DoOtherThing();
else
{
if(PromptUser())
DoSomething();
}
...except if PromptUser() fails, you also have to execute DoOtherThing(). So you need to add to your code:
if( !FlagA || !FlagB || !FlagC)
DoOtherThing();
else
{
if(PromptUser())
DoSomething();
<code> else
DoOtherThing();</code>
}
|
|
|
|
|
How is that better than what I posted?
|
|
|
|
|
Yes, he just inverted the boolean logic and made it worse in my opinion.
if(!FlagA || !FlagB) is essentially the same as if(FlagA && FlagB) but I'd rather see the latter and that's how I'd do it in my code.
Keep It Simple Stupid! (KISS)
|
|
|
|
|
It's not. Your code was how I would have done it...
|
|
|
|
|
Assuming that there is more processing going on than this example indicates, I would use a do-while-false loop:
do{
if(!flagA)break;
if(!flagB)break;
if(!flagC)break;
if(!PromptUser())break;
}while(0);
DoOtherThing();This is the sort of thing that I routinely do with COM programming where I need to check the success of pretty much every single method call. It means that I don't have a crazy amount of indenting, and it's much cleaner for me or somebody else to understand and modify later.
I can highly recommend it.
None
|
|
|
|
|
Where's the call to DoSomething?
I'm pretty sure that's not applicable to the original post.
|
|
|
|
|
Doh - I was looking at the second code listing.
Assuming that there is lots of other stuff going on between the flag tests, and there is a need for clean-up code (thus don't want to use return), I'd use the same system with a flag:
bool doSomething = false;
do{
if(!flagA)break;
if(!flagB)break;
if(!flagC)break;
if(!PromptUser())break;
doSomething = true;
}while(0);
if(doSomething)
DoSomething();
else
DoOtherThing();
None
|
|
|
|
|
Still looks like a potentially infinite loop.
|
|
|
|
|
I can assure you that it isn't!
This technique is usually called a do-once block, and commented as such to make it really obvious. Look again at just the 'loop' bit and see if you think that this block of code will still run more than once:
do{
}while(0);What you basically end up with, is a block of code that you can jump to the end of early with a 'break', without having to resort to a 'goto'. That block of code isn't a loop, as it will never run more than once. Get it?
None
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
