Avoid return statement in the middle - horror or not?
-
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:
if (guardCondition1) return guard1Default; if (guardCondition2) return guard2Default; if (guardCondition3) return guard3Default; //Do 20 lines of Complex Logic Here return result;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.
-
if ( flagA && flagB && flagC && PromptUser() )
{
DoSomething();
}
else
{
DoOtherThing();
}return ;
Keep in mind that the
&&operator is short-circuit.[Message Deleted]
-
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:
if (guardCondition1) return guard1Default; if (guardCondition2) return guard2Default; if (guardCondition3) return guard3Default; //Do 20 lines of Complex Logic Here return result;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.
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. -
[Message Deleted]
According those many sites, C and C++ do use short-circuiting. As a well-defined part of the language standard. Which means that it's fine to reply on, as long as you feel it won't impact readability. Java also seems to behave differently from what you said: http://www.student.cs.uwaterloo.ca/~cs132/Weekly/W02/SCBooleans.html[^] And anyway, it's very pointless to put extra boolean operators (&&, ||) in a language without defining them to be short-circuiting. There are | and & already, and if there is no short-circuiting they'd be the same.
-
[Message Deleted]
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(); // this may fail, and return NULL
if (p != NULL && p->SomeMethod()
....p->SomeMethod()will only be called if p is not NULL -
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:
if (guardCondition1) return guard1Default; if (guardCondition2) return guard2Default; if (guardCondition3) return guard3Default; //Do 20 lines of Complex Logic Here return result;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.
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
-
[Message Deleted]
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.
-
According those many sites, C and C++ do use short-circuiting. As a well-defined part of the language standard. Which means that it's fine to reply on, as long as you feel it won't impact readability. Java also seems to behave differently from what you said: http://www.student.cs.uwaterloo.ca/~cs132/Weekly/W02/SCBooleans.html[^] And anyway, it's very pointless to put extra boolean operators (&&, ||) in a language without defining them to be short-circuiting. There are | and & already, and if there is no short-circuiting they'd be the same.
harold aptroot wrote:
Java also seems to behave differently from what you said:
I think you are right. I was looking at this section[^] of the Java language specification:
15.7.2 Evaluate Operands before Operation The Java programming language also guarantees that every operand of an operator (except the conditional operators &&, ||, and ? : ) appears to be fully evaluated before any part of the operation itself is performed.They do explicitly exempt the logical operators. Let's hope that&&is not overloaded -- in C++ at least -- then short circuiting does not apply. Then I was reading this on C++ Sequence Points[^]. It says that"the left operand of the logical AND operator is completely evaluated and all side effects completed before continuing. There is no guarantee that the right operand of the logical AND operator will be evaluated.Of course, it doesn't say that is guaranteed not to be evaluated, either. Then it says this:"The controlling expression in a selection (if or switch) statement. The expression is completely evaluated and all side effects completed before the code dependent on the selection is executed."So, who knows. My conclusion would be that a C/C++ compiler is indeed supposed to do left-right evaluation and short circuiting. But I still say it's irrelevant. If I have to hunt that hard to get down into the bowels of how a compiler works to determine how my code will execute, I say it's a bad programming practice. I'll take these guys word for it: ...The moral is that writing code that depends on order of evaluation is a bad programming practice in any language. Naturally, it is necessary to know what things to avoid, but if you don't know how they are done on various machines, you won't be tempted to take advantage of a particular implementation.-- The C Programming Language[ -
harold aptroot wrote:
Java also seems to behave differently from what you said:
I think you are right. I was looking at this section[^] of the Java language specification:
15.7.2 Evaluate Operands before Operation The Java programming language also guarantees that every operand of an operator (except the conditional operators &&, ||, and ? : ) appears to be fully evaluated before any part of the operation itself is performed.They do explicitly exempt the logical operators. Let's hope that&&is not overloaded -- in C++ at least -- then short circuiting does not apply. Then I was reading this on C++ Sequence Points[^]. It says that"the left operand of the logical AND operator is completely evaluated and all side effects completed before continuing. There is no guarantee that the right operand of the logical AND operator will be evaluated.Of course, it doesn't say that is guaranteed not to be evaluated, either. Then it says this:"The controlling expression in a selection (if or switch) statement. The expression is completely evaluated and all side effects completed before the code dependent on the selection is executed."So, who knows. My conclusion would be that a C/C++ compiler is indeed supposed to do left-right evaluation and short circuiting. But I still say it's irrelevant. If I have to hunt that hard to get down into the bowels of how a compiler works to determine how my code will execute, I say it's a bad programming practice. I'll take these guys word for it: ...The moral is that writing code that depends on order of evaluation is a bad programming practice in any language. Naturally, it is necessary to know what things to avoid, but if you don't know how they are done on various machines, you won't be tempted to take advantage of a particular implementation.-- The C Programming Language[ -
harold aptroot wrote:
Java also seems to behave differently from what you said:
I think you are right. I was looking at this section[^] of the Java language specification:
15.7.2 Evaluate Operands before Operation The Java programming language also guarantees that every operand of an operator (except the conditional operators &&, ||, and ? : ) appears to be fully evaluated before any part of the operation itself is performed.They do explicitly exempt the logical operators. Let's hope that&&is not overloaded -- in C++ at least -- then short circuiting does not apply. Then I was reading this on C++ Sequence Points[^]. It says that"the left operand of the logical AND operator is completely evaluated and all side effects completed before continuing. There is no guarantee that the right operand of the logical AND operator will be evaluated.Of course, it doesn't say that is guaranteed not to be evaluated, either. Then it says this:"The controlling expression in a selection (if or switch) statement. The expression is completely evaluated and all side effects completed before the code dependent on the selection is executed."So, who knows. My conclusion would be that a C/C++ compiler is indeed supposed to do left-right evaluation and short circuiting. But I still say it's irrelevant. If I have to hunt that hard to get down into the bowels of how a compiler works to determine how my code will execute, I say it's a bad programming practice. I'll take these guys word for it: ...The moral is that writing code that depends on order of evaluation is a bad programming practice in any language. Naturally, it is necessary to know what things to avoid, but if you don't know how they are done on various machines, you won't be tempted to take advantage of a particular implementation.-- The C Programming Language[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. -
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
returnstatement 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 multiplereturnstatements. -
I have a coding rule - avoid "return" somewhere in the middle of a method, try using local variables to compensate instead. Observing this leads to the following (seen in existing code):
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromtUser())
{
DoSomething();
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}How I would have written this is:
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromptUser())
{
return;
}
}
}
}DoOtherThing();
I was wondering how you guys feel about it?
Thanks, Georgi
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?
--Mike-- Visual C++ MVP :cool: LINKS~! CP SearchBar v3.0 | C++ Forum FAQ I work for Keyser Söze
-
I have a coding rule - avoid "return" somewhere in the middle of a method, try using local variables to compensate instead. Observing this leads to the following (seen in existing code):
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromtUser())
{
DoSomething();
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}How I would have written this is:
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromptUser())
{
return;
}
}
}
}DoOtherThing();
I was wondering how you guys feel about it?
Thanks, Georgi
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)
-
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 executeDoOtherThing(). So you need to add to your code:if( !FlagA || !FlagB || !FlagC) // Select Variant on !Flagx or Flagx == false depending on language
DoOtherThing();
else
{
if(PromptUser())
DoSomething();
else DoOtherThing();
} -
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)
How is that better than what I posted?
-
[Message Deleted]
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.
-
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 * 5to 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. -
How is that better than what I posted?
-
How is that better than what I posted?
-
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?
--Mike-- Visual C++ MVP :cool: LINKS~! CP SearchBar v3.0 | C++ Forum FAQ I work for Keyser Söze
it all depends on how much coffe i've had for the day...
do or do not, there is no try