|
Nice technique that I often recommend to people programming in C. In my opinion the title of the tip is not the best. I would call this something like "The 'finally/except' block of procedural languages"
|
|
|
|
|
Surely anyone can program whatever he wants for himself and I am not insulted (though maybe a bit frustrated), if you continue to produce code with multiple returns. I was only pointing out, that if you want to be nice to other people by keeping your code readable, you will certainly refrain from using multiple returns.
As an example, look at the function QueryLocalGroupSIDs() in CoreCode.cpp of the codeproject article "GUI-Based RunAsEx". Clearly, a nice program and a good job. However, the cruel things the author does to the lpData2 and bSuccess variables results in two return statements inside a __finally section (which is undefined behavior during termination handling btw). Most of the code does nothing at all, because bSuccess can only be TRUE if lpData2 is not NULL. There is no point in using two return statements, because just calculating a result value and returning that at the end is always easier to read. You need 5 to 10 times more time to understand what this function actually does. I even go as far as to say, that the author did not understand his own code here, or he would not have produced so much non functional code.
Again, my point is only meant as a help for anyone, who is interested in the well being of his readers and of course, you can do with your life whatever you want 
|
|
|
|
|
Clearly we have difference experiences - and debuggers. One of my programming guidelines is to keep function length down to a single page in my editor, about 40 lines of code. I don't find it difficult to pick out one or two returns in that space. While my opinion is that judicious use of gotos or multiple returns is acceptable, neither are constructs that are prevalent in my code, and I comment heavily, so it doesn't present a problem for me. Obviously, your opinion differs. That's OK, too. Your shop, your rules.
|
|
|
|
|
The first person wanting to add a debug statement at the end will prove you wrong.
|
|
|
|
|
Sorry, but in C++ at least, not being able to add a debug statement at the end of a function is a poor reason for avoiding multiple returns. Adding such a statement which will fire in the case of early returns or exceptions is easily accomplished using an RIAA technique (e.g. create an object which will output the debug statement when it's destructed). In C, well, feel free to pick your own poison (deep nesting vs. many returns). It's a style thing, and a professional programmer ought to be able to read both styles with equal ease.
|
|
|
|
|
Multiple returns look clear only to those, who have written the code. It is a classical write-only construction. Please be kind to you readers and never ever use multiple returns. Finding bugs in non trivial code using multiple returns easily gets as hard as bad constructions using goto.
|
|
|
|
|
Never say never. Sometimes the clearest code is the shortest path through it.
|
|
|
|
|
In fact, with this pattern where the complex function is split in two functions, code that must be done at the end, must stay in the caller function.
If we really need do to a bunch of tests, then often multiple returns will give clearer code by avoiding deep nested loigic.
|
|
|
|
|
Reason for my vote of 1
People who put more than one return statement into a function or method need to rethink their life!
Just try to produce debug output at the end of a function like that or generally try to analyze what a function like that does.
|
|
|
|
|
In my opinion its OK to put return into a function especially when RAII is in action in case of local variables. Putting in more return statements often helps to avoid embedded if/else blocks that make the code less readable and longer. In my opinion those people should rethink their life who write spaghetti functions that need a lot of analyzation. Putting a breakpoint on 1-2 return statements in a short function is very easy.
|
|
|
|
|
Of course, writing spaghetti code is never a solution. Using multiple return statements however, forces the reader to trace, what the function does in order to find out, which pieces of code handle which cases. That is a lot of unnecessary analysis.
Writing your code with single returns forces the developer to think about the logic involed and therefore produces a more logic oriented, easier to read program. Given the fact, that programs are usually read an incredibly lot more often than they are written, this is a very wise investment to make.
After some time of doing that, you will find, that programming like this becomes very natural and easy and that your code will gain many other useful properties.
|
|
|
|
|
If/else blocks also force the reader to trace the code. Without multiple returns often it is hard to find out that nothing else will be executed from the function if you run into a branch, with multiple returns this is much easier. A function often starts with some sanity checks or handling simple cases and in that case its pointless for the reader to trace the if/else brances to find out that after a specified branch nothing else will execute.
In C single point of return is quite reasonable especially if you have to cleanup many resources but in C++ with RAII returning earlier often results in easier to read code that is shorter and contains less blocks/indentation. Without spaghetti functions I definitely vote for multiple returns in C++.
|
|
|
|
|
I wish the poor readers of your code good luck tracing, what that function does over and over again, just to find out, what it really does (e.g. trying to find post conditions). I will always vote for letting the author do the thinking to produce easily analysable code.
Whether or not the rest of your code is RAII is not always under your control. If you have to use library code, that isn't, your simply are out of luck. Just the same, if your function has to do something before returning, like producing a logfile entry or sending a signal. Adding that to a function using multiple returns is just ugly.
I have long years of experience with both kinds of code and have observed, which kinds of code cause which kinds of after costs. Of course you are welcome to do whatever you like, I can only advise you to really try to avoid multiple returns and maybe drop a few other bad habits while rearranging the code to avoid multiple returns and obtain a clear, logic oriented structure for a few months and you will start to understand the advantages.
|
|
|
|
|
As you see I've different observations and I use single return only in code that doesn't support RAII and/or finally (for example pascal or C). When you are writing the code of a function you are totally in control of RAII in your function and you can use RAII for whatever you want, luckily, even if you are using others' libraries. RAII is basically the "finally block" in C++. In languages with exception handling you can use finally blocks that also make returns safe (regarding to cleanup).
Summarizing: If a language has some kind of 'finally' support (RAII, finally, or a 'with-resource' construct) and you use them well then using multiple returns can be done safely. I will never understand the advantages of multiply embedded if else blocks as I've used it a lot in C but I find using it unreasonable in C++ for the reasons I've listed.
|
|
|
|
|
Ouch, now we are really drifting away from the original topic. The usage of exceptions is another hot topic we probably can talk about for ages ...
I can see you are pretty convinced about your style and you can see, that I am pretty convinced about my style, so lets just agree on not agreeing before we start throwing (pun intended) things at each other. At the end of the day, it's just important that the software does what it is supposed to do and I can only try to share the positive development I've seen in code (readability, maintainability etc.), where multiple returns have been removed. You seem to prefer the kind of spaghetti like sequences of code with lots of return statements, because they are easily written without having to think too much before hand. Whoever is still reading this should be invited to try out both and choose for themselves.
|
|
|
|
|
Wolfgang_Baron wrote: lets just agree on not agreeing before we start throwing (pun intended) things at each
... and 1-2 sentences later:
Wolfgang_Baron wrote: You seem to prefer the kind of spaghetti like sequences of code with lots of return statements, because they are easily written without having to think too much before hand.
Spaghetti and multiple returns are two different things. A function can be spaghetti with both multiple returns and with single point of return but with so many years of experience you probably know this. Please point at my sentence where I state that I like/prefer spaghetti, I think you could easily find posts where I state the opposite. Just a single or two return statements at the beginning of a function (handling for example two simple sanity checks) saves you from embedding the rest of the function into nested blocks and from writing terribly long if conditions to analyze. Could you please make my spaghetti function below more readable without multiple returns because I'm lazy to think too much:
float Func()
{
float res1 = Func1();
if (res1 < 0)
return 0;
float res2 = Func2();
if (res2 < 0)
return 0;
float res3 = Func3();
if (res3 < 0)
return 0;
return res1 + res2 + res3;
}
Wolfgang_Baron wrote: I have long years of experience with both kinds of code
I know there tere are several respectful members on codeproject with extensive experience in a lot of areas but I have never seen them reasoning in a topic with having X years of experience.
Wolfgang_Baron wrote: Whether or not the rest of your code is RAII is not always under your control. If you have to use library code, that isn't, your simply are out of luck.
Not true. This is something that makes multiple returns safe even if you return from the middle of a function. Multiple returns often happen at the beginning of a function (sanity check, simple cases) where you usually have nothing to cleanup anyway.
|
|
|
|
|
Quote: ... and 1-2 sentences later:
First the general statement, then the summary of this kind of lengthy conversation, why is that worth a quote?
Quote: Spaghetti and multiple returns are two different things.
They both have messy, hard to analyze flows of control. Throw in a few exceptions and gotos and you are ready to rumble!
Quote: A function can be spaghetti with both multiple returns and with single point of return but with so many years of experience you probably know this.
Yes, finally something we agree on . However, I am not willing to obscure the structure of my program by introducing multiple returns. I want easy to read and quickly to understand code.
Quote: Could you please make my spaghetti function below more readable without multiple returns because I'm lazy to think too much:
Well, this opens a can of worms. You created this little example for the usage of a number of functions. Maybe these functions do not provide the right interface for nicely integrating them in a function concerned with flow of control. I started to experiment with your example and found a number of ways to deal with that kind of design. The best way, of course, is to rethink your program. You will not write a beautiful part of your software, if the rest does not fit. So maybe, your low level functions should return a boolean, indicating an error, so you can chain them together with lazy evaluating || or &&. You can also write a little adapter, which collects the error value while computing the result. Modern compilers will inline that extra code, so your program will not slow down a lot. I benchmarked a lot of variants on Visual C++ 10 and gcc 4.7.3. Gcc gave the fastest execution for the following kind of code, even though it does not use lazy evaluation (I know, lazy evaluation has to be done by the functions):
typedef float vtype;
vtype FuncB1( bool & aSuccess );
vtype FuncB2( bool & aSuccess );
vtype FuncB3( bool & aSuccess );
vtype FuncF() {
vtype result;
bool l_success = true;
result = FuncB1( l_success ) + FuncB2( l_success ) + FuncB3( l_success );
if (!l_success) result = 0;
return result;
}
An example for an adapter would be:
static inline bool add_nn( vtype & aResult, vtype aValue ) {
aResult += aValue;
return aValue < 0;
}
vtype FuncH() {
vtype result = 0;
if (add_nn( result, Func1() )
|| add_nn( result, Func2() )
|| add_nn( result, Func3() ))
{
result = 0;
}
return result;
}
If you prefer lambdas and C++11 style to stow away the adapter code without a global definition, you can do:
vtype FuncI() {
vtype result = 0;
auto l_add = [&result]( vtype aValue ) -> bool {
result += aValue;
return aValue < 0;
};
if (l_add( Func1() ) || l_add( Func2() ) || l_add( Func3() )) {
result = 0;
}
return result;
}
Now, I think I spent enough time on this, so if I don't answer, I am busy doing other stuff and may not return for some time. Anyhow, feel free to continue the experiments and introduce explicit logic to your code, so that the reader has to spend less time analyzing what the code does in its entirety.
|
|
|
|
|
In simple cases, your sample codes are overkill. I think you are going too far by systematically avoiding multiple returns. There are cases, as my alternative demonstrates, where multiple returns is definitively the best solution.
On the other hand for samples like this one, if you don't have more than 3 functions, and prefer single result, then something like that would be more appropriate:
float Func()
{
float result = 0;
float res1 = Func1();
if (res1 >= 0)
{
float res2 = Func2();
if (res2 >= 0)
{
float res3 = Func3();
if (res3 >= 0)
{
result = res1 + res2 + res3;
}
}
}
return result;
}
The main problem with this solution is that it does not scale well. With more than 3 condition, it start to indent a lot and it is much harder to reoder parts than with the multiple returns solution a few replies above.
By the way, if functions are not expensive to call and the main function is not called a lot, then the following code would be adequate:
float Func()
{
float res1 = Func1();
float res2 = Func2();
float res3 = Func3();
return res1 >= 0 && res2 >= 0 && res3 >= 0 ? res1 + res2 + res3 : 0;
}
Finally, if you make the effort to to go up to the point of using lambda (or function objects), the why not make an array of those functions and call them in a loop.
Philippe Mori
|
|
|
|
|
None of the his alternatives were equivalent to the original function and the readability of his functions compared to the original and the ease of debugging (placing breakpoints to error conditions and function return values) was also far from the original code. The only equivalent transformation is your first function that is seemingly more bloated with embedded ifs and harder to understand than the original with multiple returns. In your second example I wouldn't replace an if with ?:, that way its easier to put breakpoints to the branches and it is more readable even if its a few lines more but this is just a matter of taste...
|
|
|
|
|
Modified code is not equivalent to original code but your sample of Oct. 8th.
I don't think it is necessary to stick to a particular patterns. Someone can use the most appropriate style depending on the complexity of the conditions or the debugging needs.
Like multiple returns, ?: can reduce useless verbosity but it should typically only be used in cases that are simple enough. In my last sample, someone would typically put a breakpoint on the line with the ternary operator and inspect res1, res2 and res3 and should then be able to figure out the problem if there was an error.
One advantage that I can see using distinct lines might be for code coverage tools that may works on a line per line basis.
Philippe Mori
|
|
|
|
|
We agree in this, I stated that your first func is an equivalent version of the original. A short ternary op expression doesn't make a program difficult to read.
|
|
|
|
|
Quote: In simple cases, your sample codes are overkill
We are not doing this to find a good solution for a simple case. We are looking for good patterns to write readable code, which allows the reader to quickly understand, what a function does (and thus support good code quality even after changing the function a few times) while not putting too much stress on the developer.
Quote: The main problem with this solution is that it does not scale well
Your first solution is exactly, what I would do in simple cases, but what pasztorpisti seems to dislike. Therefore, he asked for alternatives. Making a habit out of using a scalable alternative even for simple cases may be ok, if the alternative is not too complicated.
Quote: return res1 >= 0 && res2 >= 0 && res3 >= 0 ? res1 + res2 + res3 : 0;
Although I like the ?: operator more than most of the other developers I work with, making return statements complicated is not such a great idea. Always think about the poor guy, who tries to add a debug trace here. Using an extra result variable will not make your code slower, because the compiler will optimize it away and a few extra words often make the code even easier to read and a lot easier to change.
Quote: Finally, if you make the effort to to go up to the point of using lambda (or function objects), the why not make an array of those functions and call them in a loop.
That is again what I would do, if the functions all have the same interface and the values are combined in a homogeneous way. As we were doing this to find a general pattern, your code in between the calls may not allow that. You'd have to create lots of extra little functors or lambdas and that can be more difficult to read than a sequence of pieces of code. So, I'd say, the best solution just depends on what your program has to do.
|
|
|
|
|
While I would agree with most of your comments, I stronly believe that properly used, multiple returns are appropriates in some situations.
If to avoid multiple returns, you have to increase code indentation to the point that most lines are too longs (I usually break lines around 95 characters) or you extra variable and extra conditions afterward to essentially verify same thing again, early returns might be an appropriate alternative.
By the way, i usually write functions that are relatively small and thus it does not make much difference for readability, debugging or maintenance which way is used in that particular case.
In case that are very complex, it might be appropriate to put the complex code in a separate class.
Philippe Mori
|
|
|
|
|
Wolfgang_Baron wrote: First the general statement, then the summary of this kind of lengthy conversation, why is that worth a quote?
I don't know, you probably know that better. Just go back to the original post you have written.
Wolfgang_Baron wrote: They both have messy, hard to analyze flows of control. Throw in a few exceptions and gotos and you are ready to rumble!
Exceptions and gotos have nothing to do with each other, lets forget about goto.
I've just realized that your second and third function does what the original function does but do you call this readable code compared to the original one? Besides this the single point of return will work only in languages that don't have exception handling among their core features (like in java/c#) but you seemingly want to avoid discussing this. If you are a C programmer and you don't have to deal with exceptions and you don't have a finally-like language support then forcing single point of return is OK, otherwise it isn't.
|
|
|
|
|
pasztorpisti wrote: I don't know, you probably know that better. Just go back to the original post you have written.
I was questioning what you posted before that. It did not seem to make any sense. Therefore I explained what I posted, so you could clarify your reply.
Quote: Exceptions and gotos have nothing to do with each other, lets forget about goto.
They both complicate the flow of control. In fact, people used to do long jumps (non local gotos) to do some of the things people use exceptions for these days.
pasztorpisti wrote: <layer>Besides this the single point of return will work only in languages that don't have exception handling among their core features (like in java/c#) but you seemingly want to avoid discussing this. <layer>If you are a C programmer and you don't have to deal with exceptions and you don't have a finally-like language support then forcing single point of return is OK, otherwise it isn't.
I am not a C programmer and you are not obliged to use a language feature, just because the language has it. I am only saying, that avoiding multiple returns will make your code more maintainable, just like avoiding gotos (or fill in any other risky language feature of your choice). If you are forced to incorporate code, that makes use of exceptions, you will, of course, have to deal with them. You are however always free to improve the maintainability of your own code by at least trying to avoid features like exceptions, gotos or multiple returns.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
