17

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;
        }
    }
Community
  • 1
  • 1
GibboK
  • 64,078
  • 128
  • 380
  • 620
  • "if could be considered good practice" --- how do you see it? The more returns - the better? – zerkms Oct 02 '12 at 06:52
  • That second example should produce a warning about unreachable code. Which kind of answers your question. – slugster Oct 02 '12 at 06:54
  • 1
    Please see the [FAQ](http://stackoverflow.com/faq#dontask). This question doesn't really have an answer, it's completely a matter of a opinion and there are zealots on both sides of the fence. – verdesmarald Oct 02 '12 at 06:55
  • Another duplicate: [Should a function have only one return statement?](http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement) – slugster Oct 06 '12 at 02:28

11 Answers11

29

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
cuongle
  • 69,359
  • 26
  • 132
  • 196
14

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.

Guffa
  • 640,220
  • 96
  • 678
  • 956
  • Thanks Guffa for your comment! – GibboK Oct 02 '12 at 06:58
  • 6
    Why the downvote? If you don't explain what you think is *wrong*, it can't improve the answer. – Guffa Oct 02 '12 at 10:15
  • 1
    `It's easier to follow the code if there is a single point of exit.` In many cases yes, but I disagree in the general case. The other answers include several examples of code that is far easier to read with multiple return statements. – BJ Myers Feb 14 '17 at 17:40
  • The second example with "one" exit point actually has multiple exit points. If repepositoryCandidate is null or GetById() returns null or throws an exception, then you have more exit points to that simple method. By the logic that multiple exit points are "bad," then you would have to catch/swallow all exceptions to maintain a single exit point would be "good" practice. – John Sep 18 '17 at 15:14
  • 1
    I don't think there's a consensus on this the way your answer makes it seem there is. I totally disagree with this as a general principle and see people mangle code that could be much simpler just to have a single exit point. If you know you can exit early inside a loop for example, how on earth it is easier to read a flag being set, then breaking out of the loop, then rechecking the flag or returning the flag later vs just a return statement with the known result inside the loop? – Mike Marynowski Apr 22 '19 at 17:27
  • See the accepted answer here: https://softwareengineering.stackexchange.com/questions/118703/where-did-the-notion-of-one-return-only-come-from I don't know how this ever became a common "practice" in modern languages. – Mike Marynowski Apr 22 '19 at 17:35
11

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.

Jakub Konecki
  • 44,070
  • 6
  • 84
  • 125
7

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.

Konstantin Dinev
  • 30,746
  • 13
  • 64
  • 91
3

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.

akton
  • 13,492
  • 3
  • 42
  • 45
3

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. :D

Jerry Lam
  • 434
  • 1
  • 7
  • 20
2

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.

John Woo
  • 238,432
  • 61
  • 456
  • 464
2

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;
g t
  • 6,576
  • 6
  • 44
  • 83
Tigran
  • 59,345
  • 8
  • 77
  • 117
  • I just did this, changing some old code. There were few layers deep of if/else and I didn't want to add another layer, but also did not want to rewrite/restructure a lot of code. I then ran into this, checking to see if it is good practice these days to have multiple exits. There are some good points on this post. – Steve Mar 16 '18 at 18:01
1

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.

Badhon Jain
  • 177
  • 1
  • 12
1

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;  
}
Usman Khalid
  • 2,872
  • 8
  • 33
  • 63
0

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;
        }
Vale
  • 3,100
  • 2
  • 25
  • 42