Multiple returns from methods or clean code flow
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
The (professional) opinion is that humans work better with "positive" statements vs deciphering compound negatives. I will even resort to: if ( a && b && c ){ // continue. } else { return; } ... for myself. ("Early returns" probably run contrary to the notion of "diving into the code"; but since code should not run more than "a page", "diving" implies a bigger problem).
"(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
I believe I am in the minority on this but I prefer one single return statement. Multiple returns adds unnecessary confusion. For example, if you put a breakpoint near the end of a function and it never hits it may be because of the early returns so you have to go hunting around to find out what's going on. One return.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
-
I actually sometimes throw exceptions instead of just return and yes I prefer strongly the early returns. It's more modular than the nested hell of mess. Especially when multiple people add crap within a particular branch of ifs. I looked at the source code of DOOM. John Carmack uses early returns. That should settle it.
Urban Cricket wrote:
John Carmack uses early returns.
Looking at my bookshelf: The Graphics Programming Black Book, by Michael Abrash and with a foreword by John Carmack. Fearsomely thick tome, full of how to write fast graphics code. That kind of processor cycle counting on a 386 or 486 is outdated, while the algorithmic optimizations remain as current as ever, especially if you are able to delegate them to the graphics processor. Early returns as a way to waste no processor cycle too much in a function is only rarely important anymore.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
Layout can help a lot:
public bool someMethod(int valueA, bool Valueb, bool ValueC)
{
var output = DoSomethingElse(ValueA, ValueB)
? RunAnotherMethod(ValueA, output.SOmething)
: DoAnotherMethod(ValueB, ValueC)
? RunAthoerMethod(ValueB, output.SomethingElse)
: DoYetAnotherMethod(ValueA, ValueC)
...etc.
}- I would love to change the world, but they won’t give me the source code.
Still: Yuck!
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
I believe I am in the minority on this but I prefer one single return statement. Multiple returns adds unnecessary confusion. For example, if you put a breakpoint near the end of a function and it never hits it may be because of the early returns so you have to go hunting around to find out what's going on. One return.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
Putting a break point that does not get hit where you expect it to only implies that one does not understand the program flow; not that there could be "too many returns". Without a try-catch block everywhere, every exception amounts to an "early return". Also, "early returns" facilitate the returning of different "condition codes"; instead of "tramping" them along. A la IBM; 0 - Good 4 - Good with conditions 8 - Warning 16 - Oh Oh ...
"(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal
-
Putting a break point that does not get hit where you expect it to only implies that one does not understand the program flow; not that there could be "too many returns". Without a try-catch block everywhere, every exception amounts to an "early return". Also, "early returns" facilitate the returning of different "condition codes"; instead of "tramping" them along. A la IBM; 0 - Good 4 - Good with conditions 8 - Warning 16 - Oh Oh ...
"(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal
Gerry Schmitz wrote:
only implies that one does not understand the program flow;
Exactly! :-D But it's very easy to assume the code is failing down near the bottom of the function and therefore you put the breakpoint there because you don't want to step through all of it.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
-
I believe I am in the minority on this but I prefer one single return statement. Multiple returns adds unnecessary confusion. For example, if you put a breakpoint near the end of a function and it never hits it may be because of the early returns so you have to go hunting around to find out what's going on. One return.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
Nice and well, until you sit in the middle of several levels of conditions, something goes wrong and you want to get out of there. What then? Awkward nested if/else blocks? Or do we just make use of the good old GOTO to hop to your single return at the end? I do exactly that often enough in assembly programs, just because I need a single return as a point where I clean up the stack frame before actually returning. I don't really see the point if it's only a matter of principle.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
I prefer one return but if I am working with people who go ape-sunshine about it I will defer to their preference. Life is too short to wage war about that. All the while muttering under my breathe that their way leads to confusion and makes debugging more difficult like someone else already pointed out. I'm passive-agressive that way.
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
Multiple returns can lead to resource leaks if not handled properly (think of RAII)
I'd rather be phishing!
-
Urban Cricket wrote:
John Carmack uses early returns.
Looking at my bookshelf: The Graphics Programming Black Book, by Michael Abrash and with a foreword by John Carmack. Fearsomely thick tome, full of how to write fast graphics code. That kind of processor cycle counting on a 386 or 486 is outdated, while the algorithmic optimizations remain as current as ever, especially if you are able to delegate them to the graphics processor. Early returns as a way to waste no processor cycle too much in a function is only rarely important anymore.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
That is true, but it has other benefits too. Generally as a principle I prefer within a piece of code, if the task is done within a branch it should exit immediately. Later these blocks can be moved out to separate methods or classes. I work in a team where a couple of the members like to add thousands of lines of code in a single method and management tends to let us do whatever we want. So on one hand I really like being left to do what I want in terms of programming style, on the other hand they don't let me lecture others on how to code Usually I start a project, write the skeleton and after the first version they assign others to add features, which lead to those 1000 lines functions.Early returns help me with that. I just take that additional code and put it in a separate method or class :)
-
Nice and well, until you sit in the middle of several levels of conditions, something goes wrong and you want to get out of there. What then? Awkward nested if/else blocks? Or do we just make use of the good old GOTO to hop to your single return at the end? I do exactly that often enough in assembly programs, just because I need a single return as a point where I clean up the stack frame before actually returning. I don't really see the point if it's only a matter of principle.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
CodeWraith wrote:
something goes wrong and you want to get out of there
That's when you throw an exception.
CodeWraith wrote:
Awkward nested if/else blocks?
Maybe breaking things into functions would be better.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
-
Multiple returns can lead to resource leaks if not handled properly (think of RAII)
I'd rather be phishing!
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
It took me a long time to accept multiple returns. Meanwhile I love the concept "If a decision is clear, draw the consequences". And even though, if you are honest, it's a disguised goto; similar to a lot of MS code examples where a loop/break will be "missused".
It does not solve my Problem, but it answers my question
-
That is true, but it has other benefits too. Generally as a principle I prefer within a piece of code, if the task is done within a branch it should exit immediately. Later these blocks can be moved out to separate methods or classes. I work in a team where a couple of the members like to add thousands of lines of code in a single method and management tends to let us do whatever we want. So on one hand I really like being left to do what I want in terms of programming style, on the other hand they don't let me lecture others on how to code Usually I start a project, write the skeleton and after the first version they assign others to add features, which lead to those 1000 lines functions.Early returns help me with that. I just take that additional code and put it in a separate method or class :)
Such long functions can be a pain, no matter what. Early returns definitely are better to follow than deeply nested code with some code blocks being dozens or even hundreds of lines long.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
CodeWraith wrote:
something goes wrong and you want to get out of there
That's when you throw an exception.
CodeWraith wrote:
Awkward nested if/else blocks?
Maybe breaking things into functions would be better.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
ZurdoDev wrote:
That's when you throw an exception.
In what way is that better than an early return if your goal is to have a single return point? And catching it yourself at the end of your function does not justify the overhead of throwing an exception. The dreaded GOTO would then be cheaper and no more or less problematic.
ZurdoDev wrote:
Maybe breaking things into functions would be better.
Unless you can't afford to bog down the processor with too much overhead of packing parameters onto the stack or cleaning it up afterwards. On a strong processor that's no big deal anymore, but often enough I sit there with a C compiler and a small microcontroller clocked < 10 MHz. Usually some output pins need to be pulled up or down according to a certain timing, leaving me only a few machine instructions on the microcontroller within that timeframe. In that case I may have to resort to inline assembly and can't afford any function calls.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
ZurdoDev wrote:
That's when you throw an exception.
In what way is that better than an early return if your goal is to have a single return point? And catching it yourself at the end of your function does not justify the overhead of throwing an exception. The dreaded GOTO would then be cheaper and no more or less problematic.
ZurdoDev wrote:
Maybe breaking things into functions would be better.
Unless you can't afford to bog down the processor with too much overhead of packing parameters onto the stack or cleaning it up afterwards. On a strong processor that's no big deal anymore, but often enough I sit there with a C compiler and a small microcontroller clocked < 10 MHz. Usually some output pins need to be pulled up or down according to a certain timing, leaving me only a few machine instructions on the microcontroller within that timeframe. In that case I may have to resort to inline assembly and can't afford any function calls.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
CodeWraith wrote:
inline assembly and can't afford any function calls.
Different rules apply to assembly vs. OOP.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
Forogar wrote:
if (s != null && s.Thing != null)
This. Or better yet
void SomeFunction(SomeThing s)
{
if (null == s || null == s.Thing)
{
throw blah;
}
// use s here with peace of mind
} -
CodeWraith wrote:
inline assembly and can't afford any function calls.
Different rules apply to assembly vs. OOP.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
ZurdoDev wrote:
Different rules apply to assembly vs. OOP
Actually it's C code with some inline assembly, if needed. Good C code quickly borders on object orientation, just think of structs containing function pointers plus some preprocessor magic to call the 'constructors' and 'destructors'. That's essentially the way C evolved to C++. You are right, but the rules don't change by what paradigm you choose. It's more like the processor dictating which paradigm you can afford. I still have my first computer, which I built almost 41 years ago. It has a hex keyboard and I still write machine code programs for it. You would not believe how much neatly calling functions bogs down the old processor or how quickly the code to pass parameters to the functions via the stack can eat up the tiny 4k RAM. Old programs are usually a single chunk of spaghetti code, only maintainable because they can't be very large, but they shurely make more out of these limited resources.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
fail fast, fail early. end of story.
-
I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):
void SomeFunction(SomeThing s)
{
if (s != null)
{
if (s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}
}becomes:
void SomeFunction(SomeThing s)
{
if (s == null) return
if (s.Thing == null) return;
// Do Some Process with s.Thing
.
.
.
}This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:
void SomeFunction(SomeThing s)
{
if (s != null && s.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}or possibly:
void SomeFunction(SomeThing s)
{
if (s?.Thing != null)
{
// Do Some Process with s.Thing
.
.
.
}
}Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?
- I would love to change the world, but they won’t give me the source code.
Forogar wrote:
which is bad coding practice
You have to look into who says it's bad coding practice. Let's be honest: the computer doesn't care. It just some human's personal preference, which he's written down in a book. You don't have to follow anyone's personal preferences -- the computer doesn't care whose preferences you follow.
Forogar wrote:
This makes a complete hash of the natural flow of the code
OK, if you see it that way, that's your preference, and I won't try to change that. But, to me, it looks like that's a way that the computer would be happier doing it, so my preference in this doesn't match yours.
Forogar wrote:
If I was to refactor the code
If I were to refactor your statement, it would look like this:
Mark Wallace prefers:
If I were to refactor the code
Which amounts to the same thing; it's all about personal preferences. Make sure that you follow your own preferences, not those of someone who wrote a book, just because a few of his other ideas are good. ... Unless, of course, you have coding standards, where you work -- in which case, follow those religiously.
I wanna be a eunuchs developer! Pass me a bread knife!