0

Say we have a function foo(), and a bool bar. The work foo does is of no use if bar is false. What is the most proper way to write foo()?

1 

foo() {
  if(!bar)
    return;
  doWork();
}

2

foo() {
  if(bar)
    doWork();
}

Option 1 has the aesthetic advantage that doWork() (ie, the rest of the function) is not indented, but the disadvantage that if you don't look at the early return statement, you may assume that doWork() is called every time you call foo().

In general, is it bad practice to code in style 1, or should it be a personal preference?

Cory Klein
  • 51,188
  • 43
  • 183
  • 243
  • It's about personal preference. Either method is fine. – Sergey Akopov Dec 28 '10 at 17:44
  • It is personal preference...sometimes I prefer to have the returns up front...makes debugging easier as I do not have to traverse the entire method...other times I get OCD and have a single return modifying the return value as needed throughout the method. – Aaron McIver Dec 28 '10 at 17:45
  • Much bigger discussion on this available at http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement – ErikE Apr 22 '11 at 09:36

5 Answers5

2

Some people will always say to you the "have a single exit point" mantra.

Sometimes, if you need to perform a specific operation on every exit point, it makes a lot of sense. I'd say it's crucial to keep sanity, in this specific case.

Now, if you doesn't have this need, I, personally, see no problem in just exiting as soon as you can and keeping the code on level of ident lower.

I've seen people wrapping the whole code in a do { ... } while (0); block just to keep the single exit point rule, using a break instead of a return. It drives me crazy. But it can be a useful device in some situation.

Overall, use common sense and use what makes more sense in your specific problem.

Vitor Py
  • 5,145
  • 4
  • 39
  • 62
  • The times when you need to perform operations at every exit point is one the times when goto is really useful. Just stick a cleanup: label at the end of the function and jump to it. Much less obfuscatory than a do/while{0}. – frankc Dec 28 '10 at 17:57
  • @frankc I do agree. I believe code should always tell the "truth". If it needs a goto, do a goto instead of disguising it with a do/while{0}. I was just pointing it out that doing this is a somewhat common practice. – Vitor Py Dec 28 '10 at 18:02
  • You get the answer as this explained the underlying question I had, and explained more about the "single exit point" mantra, and that it is more than an always-follow or never-follow guideline. – Cory Klein Dec 28 '10 at 18:04
2

Style 1 is very useful for guarding statements. Like this:

void work() {
  if (!something)
     return;

  //do the job
}

Otherwise, I would say it depends on the situation. If the if is tightly connected with the following logic, I will use style 2, otherwise I will usestyle 1. To summarize: always use the one which makes your code more cleaner and readable.

Petar Minchev
  • 46,889
  • 11
  • 103
  • 119
1

My two cents' worth: If you keep your functions small, multiple returns aren't a real issue. In large functions (which probably should be refactored, but sometimes aren't), multiple return statements--especially from within nested control structures--start to behave like gotos, making the function more difficult to reason about.

Brian Clapper
  • 25,705
  • 7
  • 65
  • 65
0

The proper way to code a function is to have a single entry point and a single exit point. Doing this makes it easier to maintain and debug an application. Based on your examples, you should write code using the second style that you presented:

foo() {
  if(bar)
    doWork();
}
k rey
  • 611
  • 4
  • 11
  • 1
    I really don't believe in the single exit point mantra. Sometimes you just mess up your code to keep it. – Petar Minchev Dec 28 '10 at 17:47
  • From a single developer perspective it may not make much of a difference. When working in a team, or building a reusable library, this principle can have a significant impact on debugging and maintainability. By not following this principle it is easier to introduce memory leaks, maintain locks and hold files and connections open. In addition, if the function has to be refactored to return a value it is much easier to have a single exit point. It is not a hard and fast rule, but one that should be favored over the alternative. – k rey Dec 28 '10 at 18:02
0

This a personal preference, and very subjective - you're likely to see any number of opinions on this. IMHO, there is nothing wrong with either style. Each displays control flow in a reasonable manner, and option 1 can actually be very useful in checking certain function parameters, etc.

goric
  • 11,491
  • 7
  • 53
  • 69