Method exit points vs. indentation
-
In the back of my head, I have some rule that says each method should have exactly one entry point and exactly one exit point. I don't remember the context/relevance though. So my question: Which one of the two artificial examples below would you prefer and why? Method with one entry/exit point with indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo >= 0)
{
// some more actions on foo.
if (foo * foofoo <= 2000)
{
// some more stuff.
}
}
}Or multiple exit points with no indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo < 0) return;
// some more actions on foo.
if (foo * foofoo > 2000) return;
// some more stuff.
}Please ramble away... // Edit: Typo.
-
In the back of my head, I have some rule that says each method should have exactly one entry point and exactly one exit point. I don't remember the context/relevance though. So my question: Which one of the two artificial examples below would you prefer and why? Method with one entry/exit point with indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo >= 0)
{
// some more actions on foo.
if (foo * foofoo <= 2000)
{
// some more stuff.
}
}
}Or multiple exit points with no indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo < 0) return;
// some more actions on foo.
if (foo * foofoo > 2000) return;
// some more stuff.
}Please ramble away... // Edit: Typo.
Given the length of the function, I prefer the first one. If the function gets to be much more than 30 lines or so, I will do the 2nd one. Here's a variation on the first one that allows more convenient debugging (set breakpoints at each
if (bValid)
line:void SomeMethod()
{
int foo = bar();
int foofoo = barbar();bool bValid = (foo >= 0); if (bValid) { some actions on foo } bValid = (foo \* foofoo >= 2000); if (bValid) { some more stuff }
}
I would also return a
bool
orint
because if your don't, there's no way to determine if the function succeeded or not."Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001 -
Given the length of the function, I prefer the first one. If the function gets to be much more than 30 lines or so, I will do the 2nd one. Here's a variation on the first one that allows more convenient debugging (set breakpoints at each
if (bValid)
line:void SomeMethod()
{
int foo = bar();
int foofoo = barbar();bool bValid = (foo >= 0); if (bValid) { some actions on foo } bValid = (foo \* foofoo >= 2000); if (bValid) { some more stuff }
}
I would also return a
bool
orint
because if your don't, there's no way to determine if the function succeeded or not."Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001John Simmons / outlaw programmer wrote:
I would also return a bool or int because if your don't, there's no way to determine if the function succeeded or not.
Depends. If it's usual for method to fail, sure. If it's not, I'd go with exceptions.
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne Metcalfe -
John Simmons / outlaw programmer wrote:
I would also return a bool or int because if your don't, there's no way to determine if the function succeeded or not.
Depends. If it's usual for method to fail, sure. If it's not, I'd go with exceptions.
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne MetcalfeGiven the little amount of info we got, an exception would be completely unreasonable here.
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001 -
Given the little amount of info we got, an exception would be completely unreasonable here.
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001If method attempts to do something, and fails, return code is reasonable. If method is supposed to do something, and fails, exception is reasonable. Given the little amount of info we got, isn't it 50/50?
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne Metcalfe -
If method attempts to do something, and fails, return code is reasonable. If method is supposed to do something, and fails, exception is reasonable. Given the little amount of info we got, isn't it 50/50?
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne MetcalfeBut we're not even sure what language this is. It could be C.
Kevin
-
But we're not even sure what language this is. It could be C.
Kevin
C doesn't have
bool
:)
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne Metcalfe -
In the back of my head, I have some rule that says each method should have exactly one entry point and exactly one exit point. I don't remember the context/relevance though. So my question: Which one of the two artificial examples below would you prefer and why? Method with one entry/exit point with indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo >= 0)
{
// some more actions on foo.
if (foo * foofoo <= 2000)
{
// some more stuff.
}
}
}Or multiple exit points with no indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo < 0) return;
// some more actions on foo.
if (foo * foofoo > 2000) return;
// some more stuff.
}Please ramble away... // Edit: Typo.
IMHO I really think it depends on the context, or more specifically, the specific logic involved. Ordinarily I would go with the first one, but if, however, I'm doing something in which I need to remove special cases, like vector normalization, for instance, which is impossible only if all components of that vector are zero, I'd write an if statement which would execute only in that special case, and throw and exception therein (and exiting the function there, as a result). Anyway, that's just me. I'm sure it depends on what some one's been trained to do.
-Gatsby
-
C doesn't have
bool
:)
[My Blog]
"Visual studio desperately needs some performance improvements. It is sometimes almost as slow as eclipse." - Rüdiger Klaehn
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne MetcalfeC99 does. :)
Kevin
-
C99 does. :)
Kevin
Thank you for your replies. I agree on your points of view regarding methods should usually have a return type. By the way I don't think return types and exceptions exclude each other. Especially when a method has a return type that is not a standard variable like bool or int but an object of some kind, I prefer to throw an exception instead of returning null when the method fails. However, I don't think it really matters for the exit point vs. indentation question, even though exceptions would also be exit points. Anyways, I disregarded that for the sake of simplicity.
-
Thank you for your replies. I agree on your points of view regarding methods should usually have a return type. By the way I don't think return types and exceptions exclude each other. Especially when a method has a return type that is not a standard variable like bool or int but an object of some kind, I prefer to throw an exception instead of returning null when the method fails. However, I don't think it really matters for the exit point vs. indentation question, even though exceptions would also be exit points. Anyways, I disregarded that for the sake of simplicity.
Yes, we were going off on a tangent from the original question. My own approach is to opt for a single return unless I have a good reason not to. That's a bit vague but often it's cleaner to do a condition check at the beginning and return straight away if not satisfied. But I wouldn't go for multiple returns scattered throughout a long function. OTOH, I try to avoid long functions. I did once maintain some code which tried religiously to stick to the single return principle and this introduced a bug. Relaxing the rule would have been OK. Re: exceptions, I take my approach from "design by contract" philosophy: if a method cannot fulfil its contract, i.e., cannot do what it's supposed to do then it should throw an exception. In a language with exception handling return values should not be used to indicate logic errors. Like you I dislike returning null objects for failure. The problem is that when you're maintaining code it's often difficult to tell whether nulls are expected behaviour or not. Often they are validly expected but equally often the author of the code does not make this clear! I hope very much that the contract programming ideas in Spec# Spec#[^] make it into C# 4.0. A recent MS chat hints that they might.
Kevin
-
In the back of my head, I have some rule that says each method should have exactly one entry point and exactly one exit point. I don't remember the context/relevance though. So my question: Which one of the two artificial examples below would you prefer and why? Method with one entry/exit point with indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo >= 0)
{
// some more actions on foo.
if (foo * foofoo <= 2000)
{
// some more stuff.
}
}
}Or multiple exit points with no indentation:
void SomeMethod()
{
int foo = bar();
int foofoo = barbar();
if (foo < 0) return;
// some more actions on foo.
if (foo * foofoo > 2000) return;
// some more stuff.
}Please ramble away... // Edit: Typo.
The first one, every time. Unfortunately,
throw
andyield
muck things up a bit. However, I don't consider athrow
to be an exit point. (Convenient, huh?) :-D