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.
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!
-
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.
ZurdoDev wrote:
One return.
There is only one return, from the program-flow perspective: the one it hits first. When a program is running, the number of lines of code that are read varies according to loops and if statements. Having lots of quickly finished loops and quick escapes from if blocks is like hitting green lights all the way down the road. Save your processor a billionth of a second of effort: let it get out as soon as it knows it has to get out.
I wanna be a eunuchs developer! Pass me a bread knife!
-
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'm definitely in the single exit camp. And, after browsing this thread, finding it very hard to argue the case for it beyond "'cause I am, that's why!" I just like knowing there's a single exit point where I nuke that pointer if it's non-null etc.
"Look here you function - or threadproc - or whatever it is you call your self, you're not getting out of here until your hair is combed and you've buttoned your shirt."
If I were to use the multiple return paradigm I think I'd use multiple
goto exit's instead. Which seems kinda uglee. I'm definitely in the single exit camp. 'cause I am, that's why. -
I'm definitely in the single exit camp. And, after browsing this thread, finding it very hard to argue the case for it beyond "'cause I am, that's why!" I just like knowing there's a single exit point where I nuke that pointer if it's non-null etc.
"Look here you function - or threadproc - or whatever it is you call your self, you're not getting out of here until your hair is combed and you've buttoned your shirt."
If I were to use the multiple return paradigm I think I'd use multiple
goto exit's instead. Which seems kinda uglee. I'm definitely in the single exit camp. 'cause I am, that's why.Tim Deveaux wrote:
after browsing this thread, finding it very hard to argue the case for it beyond "'cause I am, that's why!"
That's a good enough reason. Me, I see adding returns where they fit as being more efficient (which it is), but unless you're doing something really intensive like editing high-res game graphics or video (where loops and if-blocks are hit, quite literally, billion of times), it won't make a difference that's human-noticeable, so stick to what you're happy with, and what makes your code easier on your eye, when you have to revisit it.
Tim Deveaux wrote:
If I were to use the multiple return paradigm I think I'd use multiple
goto exit's instead. Which seems kinda uglee.Every jump to a non-sequential line is a goto. Loops and if statements were invented to save you the trouble of writing endless goto lines, by adding them for you, in the background behind the code. Think: What does return do that goto doesn't? (Answer: it satisfies the anti-goto evangelists, by using a function named "return", which does nothing but call goto.) Saying that the goto is unacceptable is saying that
ifandforare unacceptable. Never be afraid of using a goto in sequential code, as long as you use it intelligently. E.g. exiting an if-block with a goto is usually fine, but exiting a loop with a goto often isn't (unless you're only using global variables, which... Yeah, no need to expound on that one).I wanna be a eunuchs developer! Pass me a bread knife!
-
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's an old lesson I learned, probably from the days of assembly -- always have one point of return, mainly for consistent stack cleanup. I do rarely make an exception (to that rule) but usually end up making some other change that removes the if. If you're doing parameter checking, as in your example, I tend to think it's better to throw an exception -- why should the function that's being called expect anything but valid parameters? I've seen
returnsprinkled throughout a function as part of the flow control. I hate that. Sometimes I don't see the return, set the breakpoint at the end of the function, and then have to steps through from the top and realize some moron tossed in an early return. I'd almost rather they use agototo the return, haha. Personally, I look at code like that and refactor it into smaller functions that have noifstatements, and the flow control occurs in a higher level function that doesn't do anything but call other functions based on conditions of previous functions or the data values. A lot more readable too when you separate out the flow control from the individual activities of each flow.Latest Article - Slack-Chatting with you rPi Learning to code with python is like learning to swim with those little arm floaties. It gives you undeserved confidence and will eventually drown you. - DangerBunny Artificial intelligence is the only remedy for natural stupidity. - CDP1802
-
Tim Deveaux wrote:
after browsing this thread, finding it very hard to argue the case for it beyond "'cause I am, that's why!"
That's a good enough reason. Me, I see adding returns where they fit as being more efficient (which it is), but unless you're doing something really intensive like editing high-res game graphics or video (where loops and if-blocks are hit, quite literally, billion of times), it won't make a difference that's human-noticeable, so stick to what you're happy with, and what makes your code easier on your eye, when you have to revisit it.
Tim Deveaux wrote:
If I were to use the multiple return paradigm I think I'd use multiple
goto exit's instead. Which seems kinda uglee.Every jump to a non-sequential line is a goto. Loops and if statements were invented to save you the trouble of writing endless goto lines, by adding them for you, in the background behind the code. Think: What does return do that goto doesn't? (Answer: it satisfies the anti-goto evangelists, by using a function named "return", which does nothing but call goto.) Saying that the goto is unacceptable is saying that
ifandforare unacceptable. Never be afraid of using a goto in sequential code, as long as you use it intelligently. E.g. exiting an if-block with a goto is usually fine, but exiting a loop with a goto often isn't (unless you're only using global variables, which... Yeah, no need to expound on that one).I wanna be a eunuchs developer! Pass me a bread knife!
Mark_Wallace wrote:
... stick to what you're happy with
Yes - in that it's a matter of style. If I was working on a codebase that used the fast exit religiously, I'd stick to it - better not to mix styles. But my style it definitely ain't - and that's part of it - the reassurance that if I wrote this my way it exits here - which is worth more to me than the (alleged) increase in readability.
-
Whitesmiths.
Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!
-
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.
agreed. that is just a nasty example of embedded logic.
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
-
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 think there are several issues at play here, the most important is one of consistency. If someone coming behind you can pick up on your style, much of the griping goes by the wayside. I am a strong supporter of early return - range checking arguments and the like. Where you get in to trouble is a 500 line function with nested returns - OR - so many levels of logic (to avoid nested returns) that it's screaming at you to re-factor it....
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
-
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.
Not messy at all. When reading through a method line-by-line
if SomeErrorCondition then
Exit(ErrorCode);is actually pretty darn readable. When debugging, setting one breakpoint at the single return statement and then fiddling which branch of the nested if-statement was taken isn't really easier than putting a breakpoint at every return statement and see which gets hit.
-
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 think that the Return statement should be dropped in any language, for example look at Delphi (object pascal) they did not have a return statement until recently it seems. A function had to fill up a variable called Result, and because there was no return statement developers where forced to maintain a clean flow. This is how it should be everywhere IMHO
-
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:
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?
For those situations, I loved the exit mechanism in the CHILL language: Any composite statement could be labeled (a function body was a composite statement labeled by the function name). The label did not identify a "point" in the code, but the entire composite. So you could exit any composite statement by "EXIT label". (Another nice use of the label: You could add it to the termination of the composite statement, any sort of END statement, easing the reading of deeply nested code, and the compiler would check the label to make sure that you made no mistakes in the nesting.) Unfortunately, CHILL never got out of telephone switch programming (for which it was developed). It really was a nice language in a lot of respects.
-
Under most circumstances, a single return is best. But ... I much prefer this:
void MyFunc ()
{
if (!ValidateUserInput(ATextBox.Text))
{
TellUserAboutTheProblem();
return;
}
if (!ValidateUserInput(AnotherTextBox.Text))
{
TellUserAboutTheProblem();
return;
}
...
}To this:
void MyFunc ()
{
if (!ValidateUserInput(ATextBox.Text))
{
TellUserAboutTheProblem();
}
else if (!ValidateUserInput(AnotherTextBox.Text))
{
TellUserAboutTheProblem();
}
else
{
...
}
}And sometimes the best thing to do is just return, particularly from a nested loop:
for (int i = 0; i < 100; i++)
{
for (int j = 0; j < 100; j++)
{
if (myArray[i, j] == value) return true;
}
}
return false;Any other mechanism is just making it more complicated, not less.
Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!
-
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'm not sure which version of ReSharper you're using, but modern versions of Rider (which use ReSharper via IPC) would have recommended an early return with the ?. syntax you're proposing, granting you both the shorter expression and shallower nesting, which is good for readability. Early returns do make the code easier to grok, if for no other reason than that it looks less like a giant chevron heading off into the distant future.
-
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.
IMHO Early returns can simplify code and help remove 'blanket cases' rather than have deep nested logic which can be hard to read at first glance. Ive done this for getting on for 4 decades and it's never hurt me or the code ive written, and as for 'horribly messy code', if it's done right then i disagree
-
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 like single returns but there are exceptions. Sometimes checking needs to be done at the start of a function. Checking that needs a couple of lines, not a single
if. Sometimes there are 2-3 such checks. The single-return-rule would force the entire function to be indented 2-3 extra levels. Multi-return: zero extra levels."If we don't change direction, we'll end up where we're going"
-
ZurdoDev wrote:
One return.
There is only one return, from the program-flow perspective: the one it hits first. When a program is running, the number of lines of code that are read varies according to loops and if statements. Having lots of quickly finished loops and quick escapes from if blocks is like hitting green lights all the way down the road. Save your processor a billionth of a second of effort: let it get out as soon as it knows it has to get out.
I wanna be a eunuchs developer! Pass me a bread knife!
Mark_Wallace wrote:
Save your processor a billionth of a second of effort
Depending on the processor and the stack protocol you use to pass parameters and return values, you may find that you gain little to nothing. Been there recently when I had to modify the 'traditional' call protocol of my old computer. Now it uses a second stack to pass parameters and return values, instead of inlining the parameters with the code for the call. Yuck, was way to close to self modifying code!. And I extended the address of the subroutine to 24 bits, so that I now can do bank switching in the calling protocol and call code anywhere in a 16 Mb memory space without the processor noticing anything. Not bad for a 40 year old 8 bit computer. If only there was a convenient way to access data anywhere in that memory space as well.
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 thought you were joking when you said Whitesmiths!
for()
{
for()Really hurts my eyes :sigh:
for()
{
for()I like.
"If we don't change direction, we'll end up where we're going"
You are welcome to your opinion*, but I like the way it's consistent: the indentation is the whole of the relevant block of code:
if (a)
b;if (a)
{
b;
c;
}Instead of the inconsistent Allman:
if (a)
b;if (a)
{
b;
c;
}* As long as your opinion doesn't include using 1TB, of course.
Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!
-
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.
Years ago I was working with people who insisted on single point of return and single loop termination, with no exception whatsoever. Both continue and break out of loops were "forbidden". These were also people who insisted on putting opening and closing braces on separate lines, and always enclose the body of an if in braces, even a single assignment (so the minimum line count for an if statement was four, eight lines for an if/else). In some code, opening and closing braces made up at least a third of the code lines. I guess that made me stall. I got sick of vading through jungles and fords of little more than braces. Finding the end of a loop, or even a function, required you to leaf through pages by pages of code with minimal information content. In my first programming course, one basic principle was taught: Always fit a function in a single page, so that you can overview all of it. Obviously, the main message was to choose a suitable abstraction level and factor out common sub-operations, but if an if/else costs you a minimum of eight lines, you can't build much abstraction (for the next level) in a single page! So I use breaks, continues and returns, to keep the function logic together, not spread over multiple pages / screenfuls. If you immediately see where the loop ends, or you have all the returns on a single page, you will easily manage it. If you like to water out your code with tons of braces and elses and umpteen nesting levels, then you loose control over your returns. But that is exactly what returns and continues and breaks are meant to avoid.
