Possible Duplicate:
Why is it good practice to return at the end of a method
I would like to know if could be considered good practice use several RETURN statements in a method and why.
If not, I would like to know how you would rewrite the code in a different way.
public string GetNominativeById(int? candidateId)
{
if (candidateId.HasValue)
return repepositoryCandidate.GetById(candidateId.Value).Nominative;
else
return string.Empty;
}
}
With one RETURN
public string GetNominativeById(int? candidateId)
{
string result;
if (candidateId.HasValue)
result = repepositoryCandidate.GetById(candidateId.Value).Nominative;
else
result = string.Empty;
return result;
}
}
Answers:
Thank you for visiting the Q&A section on Magenaut. Please note that all the answers may not help you solve the issue immediately. So please treat them as advisements. If you found the post helpful (or not), leave a comment & I’ll get back to you as soon as possible.
Method 1
You don’t actually need else
string GetNominativeById(int? candidateId)
{
if (!candidateId.HasValue)
return string.Empty;
return repepositoryCandidate.GetById(candidateId.Value).Nominative;
}
Consider this anti arrow pattern:
if (condition1)
{
if (condition2)
{
if (condition3)
{
// code lines
}
}
}
the way to return immediately will make your code more readable:
if (!condition1) return; if (!condition2) return; if (!condition3) return; // code lines
Method 2
No, it’s considered bad practice not so good practice to have multiple exit points in a method. It’s easier to follow the code if there is a single point of exit.
However, when the method is as small as in the example, it’s not hard to follow the code anyway, so having multiple exit points is not really a problem. If it makes the code simpler, you can very well use multiple return statements.
Method 3
Although you should strive to have only one return statement for readability purposes the are several patterns that involve multiple return statements. One example is Guard Clause.
Example of guard clause:
public Foo merge (Foo a, Foo b) {
if (a == null) return b;
if (b == null) return a;
// complicated merge code goes here.
}
Some style guides would have us write this with a single return as follows
public Foo merge (Foo a, Foo b) {
Foo result;
if (a != null) {
if (b != null) {
// complicated merge code goes here.
} else {
result = a;
}
} else {
result = b;
}
return result;
}
Another case is a switch statement when you might want to return from each case:
switch(foo)
{
case "A":
return "Foo";
case "B":
return "Bar";
default:
throw new NotSupportedException();
}
I would rewrite your code as:
public string GetNominativeById(int? candidateId)
{
return candidateId.HasValue
? repepositoryCandidate.GetById(candidateId.Value).Nominative;
: string.Empty;
}
At the end of the day, remember that you (and other developers) will be reading your code many times, so make sure that it’s readable and obvious.
Method 4
Take a look at the following Article on Wikipedia. What you’re asking is whether you should follow the SESE (single entry, single exit) principle from structured programming.
Method 5
One of the rules of Structured programming states that each method should have a single entry and exit point. Having a single exit point (return statement in this case) means any clean up, such as calling Close or Dispose, only needs to happen once. The impact of having multiple exit points is minor on small methods but increases as method complexity increases, where it may be easy to miss a case, or as code as refactored or modified.
Method 6
there is nothing wrong with having more return statement, sometimes it actually help you minify your code and save some unnecessary variable assign like what Cuong Le did pointed out. 😀
Method 7
make it a habit to add return at the end of the method, so you will have to close any active objects (if you have)
public string GetNominativeById(int? candidateId)
{
string _returnValue = string.Empty;
if (candidateId.HasValue)
_returnValue repepositoryCandidate.GetById(candidateId.Value).Nominative;
else
_returnValue = string.Empty;
return _returnValue;
}
side-note: Ternary Operator is not really an answer on this (I think) because there are some case that you multiple code blocks in your IF statement.
Method 8
All this depends on
- coding standard you use with other developers
- and actual code readability (so personal perception of the code)
In general, when if/else becomes too much, I use return instead.
So instead of using:
if(...)
{
if(...)
{
if(...)
{
}
}
else if(...)
{
}
..
else
{
}
}
use a return:
if(!...) return; if(!...) return;
Method 9
you actually can not use more than one return statement in a single method, what you did in your code, you used a if else statement, so only one will be executed anyway. Your code seems good to me.
Method 10
Yes if it is necessary then why not use several return statements. There will be no issue with the performance.
To rewrite this code:
public string GetNominativeById(int? candidateId)
{
if (candidateId.HasValue)
return repepositoryCandidate.GetById(candidateId.Value).Nominative;
return string.empty;
}
OR use “ternary operator”.
public string GetNominativeById(int? candidateId)
{
return candidateId.HasValue ? repepositoryCandidate.GetById(candidateId.Value).Nominative : string.Empty;
}
Method 11
Why not? But you don’t need else.
public string GetNominativeById(int? candidateId)
{
if (candidateId.HasValue)
return repepositoryCandidate.GetById(candidateId.Value).Nominative;
return string.Empty;
}
All methods was sourced from stackoverflow.com or stackexchange.com, is licensed under cc by-sa 2.5, cc by-sa 3.0 and cc by-sa 4.0