Good programming habits with return statements

Go To StackoverFlow.com

2

I'm a beginning programmer studying computer science and I've been told many times that using a return statement in a loop is not a good programming habit and should be avoided. Nowadays, I read a lot of code in the Java library to learn how everything works and I'm surprised to see that return statements in loop are everywhere.

Does anyone could explain me why using return statements in loops should be avoided?

Thanks!

2012-04-03 20:01
by stzzz1
http://stackoverflow.com/q/4805258/10468 might be worth a loo - DarenW 2012-04-03 20:03
thank you for the link - stzzz1 2012-04-03 20:34


7

Most people like to look at a method and assume a single point of exit. When you interpolate returns in the middle of loops it breaks up the flow a bit and jars this assumption. That's why many will make the "not clean" argument. There's really nothing dirty about it, but it forces the "human" reader of the program to dig deeper into the method to get a clearer understanding of its flow. Anything you can do to reduce the amount of mental deducing people have to do to understand the flow of your logic is considered a good thing.

If I could glance at a method without reading through its internals while getting an overall general understanding of what it does and where it exits then the author did a good job. Trouble arises when a method, which means well initially, grows over time and forces maintainers to juggle multiple states mentally in an exercise to understand it.

A good example is the classic guard clause which is considered good practice by some. It's a way of checking the state or a parameter for validity before continuing. If a method is given the wrong stuff then we don't want to continue. See this fake code:

public void doSomethingCool(Object stuff) {
  if(weWereNotExpectingThis(stuff)) return; //guard clause

  while(somethingHappens()) {
      doSomethingCool();
  }
  return;
}

Somebody comes along and says, "hey cool! We it's ok to use return statements in the middle of methods!" They make changes and it becomes:

public void doSomethingCool(Object stuff) {
  if(weWereNotExpectingThis(stuff)) return; //guard clause

  while(somethingHappens()) {
    if(somthingHasChanged()) return; //because my team likes early exits, Yay!
    else {
      doSomethingCool();
    }
  }
  return;
}

Still not too shabby, but more and more changes go in and you end up with this:

public void doSomethingCool(Object stuff) {
  if(weWereNotExpectingThis(stuff)) return; //guard clause

  while(somethingHappens()) {
    if(somthingHasChanged()) return;
    else if(weDetectNewInput()) {
      doSomethingCool();
    } else {
      doSomethingBoring();
      if(weDetectCoolInput()) {
        doSomethingCool();
        continue;
      }
      return;
    }
  }
  return;
}

Now you see where this is going. It is difficult to understand where we jump or what's going on at a single glance.

2012-04-03 20:23
by Cliff
thank you for this great answer - stzzz1 2012-04-03 21:11
+1 Single return statement is the only correct answer for so many reasons - Adam 2012-04-03 21:16
if you use the same constructs as with guard statements your while loop will not look so complicated - user1944408 2014-01-27 15:57
I agree but the problem is with the "if" which assumes that others will have the discipline to maintain "the same constructs". Most engineers (even well meaning engineers) will see a method constructed with multiple exits and just follow form. If the method is refactored to convey its intent w/o multiple exits then there is a mental hurdle you'd have to overcome to be the 1st to introduce an early exit point. so long as the for of guard clause at the very top is followed with only one other exit at the bottom then it can be managed - Cliff 2014-01-27 17:59
it definitely depends on your habits. For me repeatedly using returns doesn't show the control-flow and it's depth. It's often hard to get in which case something happens and when not - brainray 2015-04-01 09:54
I find it actually much easier to understand with all the returns and continue here. If you translate that into "single exit" code, you'll end up with a complicated loop with a lot of if and else setting variables here and there, and, at your single exit point, you have no idea about the state of all those variables - Joffrey 2016-04-01 15:49
@Joffrey I believe you find this more comfortable and familiar to your style. If you compare the 3rd example to the 1st example you will understand where I was going with my point. The 1st example is simple, less code. The last example involves branches which you must mentally map. A 4th more practical example would simplify the structure rather than adding more jumps/branches to it. The simpler example would extract the nested blocks into easy to follow methods - Cliff 2016-04-01 16:07
@Cliff Well we can't compare these examples, because they don't do the same thing. If we had the corresponding single-exit versions (and especially with a method returning a value), we could compare. Either way, the 3rd example should not contain all this logic, and it should be split into other methods/functions as you said. Then the global function would stay readable whether you use a single-exit or not - Joffrey 2016-04-01 17:15


6

It all boils down to "clean code". That statement means different things to different people. Having one return statement at the bottom of a method is considered clean; that is subjective.

That said, methods can have multiple return statements and still be considered clean. It all depends on the tolerance of you and your team, and more importantly, your senior developers consider clean.

It's hard to quantify when you cross the line between acceptable return habits and unreadable/bug-prone code. A method that is arranged as follows

method {

 if(!x) return;

  for(...) {
    if (x && y) return

    for(...) {
        if (z) return;
    }
  }

  if (a) return;

  ...
  return;
}

is pretty much not clean no matter how you look at it.

In other words, too many return statements is a code-smell that possible the code in question needs another look

2012-04-03 20:03
by hvgotcodes
thanks for your answer, it is very helpful - stzzz1 2012-04-03 20:35


2

The reason return statements in loops have long been discouraged is because of a paradigm called structured programming, which basically means that the flow of execution within a program should be simple and easily followed from anywhere. At the time, GOTO was still in widespread use and was making a nightmare of code readability; as part of the backlash against it, it was often said that every block of code should have only one exit point, and the only jumps should be in conditional branches, loops, and function calls. Many people still believe this, and consequently dislike break statements, exceptions, and return statements in loops.

My own view is that it's more important to be able to understand what a particular chunk of code does than to be able to follow its exact flow of execution. (I often program in functional languages, and try to write my code in a functional style even when I'm not.) As such, I think return statements in loops are fine. It's ultimately a matter of taste.

2012-04-03 20:13
by Taymon
thank you, it makes a lot of sense - stzzz1 2012-04-03 20:37


2

Since I can't yet comment on other answers, I'm forced to post a new one. I would be very careful about saying that any philosophy is "always" correct. It's all about trade-offs. If adding a second return statement helps you to avoid unnecessary use of break statements and boolean flags to manage your program flow, it is probably the right thing to do.

However, others commenters are correct in suggesting that a web of return statements, breaks, and flags are a "code smell" -- see if your methods can be broken down into smaller, more modular pieces.

2012-04-03 21:54
by ach


1

Those who adhere to the "single return" religion will go to great lengths to break out of loops cleanly, setting flags to indicate that it is time to return.

The best way to take such admonitions is to think about whether there is a good reason to return prematurely, or whether it is indicating sloppiness in thinking about how to structure the code.

2012-04-03 20:05
by DRVic


1

If the objective is clean code and readability, IMHO the best approach is quit a function on the spot i.e. return in any place meeting a condition instead of having a single return in the end.

And this is perhaps not obvious in trivial examples of functions but in functions that do really complicated work, for me 1)it is easier to understand what is going on 2) it is easier to read 3) IMO it is less error prone- many times there has been a bug detected and a fix is needed ASAP. If you keep one return in the end, I have seen many times that a "quick and dirty" fix may break the code.

But this is my personal opinion.

2012-04-03 21:00
by Cratylus


0

In the olden days it could confuse compilers and the VM, not so much anymore so it's generally a safe practice. I think people also make the general 'poor practice' argument that if your a truly good programmer you'll find a more elegant way to do the same thing.

Cheers

2012-04-03 20:03
by RyanS


0

I think the reason many people consider it cleaner ( and safer style ) is; as functions get more and more complicate ( which they often do as applications grow ), exit points in the middle of the function can be harder to trace, can become confusing and can lead to subtle bugs.

As with all 'rules' though, there are exceptions and cases where a return in the middle of a function is way cleaner than the likely if ( !error ) blocks that might result from waiting until the end.

2012-04-03 20:06
by StAlphonzo
On the other hand: people jump through a lot of hoops and create unreadable code just so they can have a single return - Mark Rotteveel 2012-04-03 20:36