return in for loop or outside loop

Question

Today, someone attended me to bad use of the return keyword in Java. I had written a simple for loop to validate that something is in an array. Supposing array is an array of length n, this was my code:

for (int i = 0; i < array.length; ++i) {
    if (array[i] == valueToFind) {
        return true;
    }
}
return false;

Now someone told me that this is not very good programming because I use the return statement inside a loop and this would cause garbage collection to malfunction. Therefore, better code would be:

int i = 0;
while (i < array.length && array[i] != valueToFind) {
    ++i;
}
return i != array.length;

The problem is that I can't come up with a proper explenation of why the first for loop isn't a good practice. Can somebody give me an explanation?

Solution 1

Now someone told me that this is not very good programming because I use the return statement inside a loop and this would cause garbage collection to malfunction.

That's incorrect, and suggests you should treat other advice from that person with a degree of skepticism.

The mantra of "only have one return statement" (or more generally, only one exit point) is important in languages where you have to manage all resources yourself - that way you can make sure you put all your cleanup code in one place.

It's much less useful in Java: as soon as you know that you should return (and what the return value should be), just return. That way it's simpler to read - you don't have to take in any of the rest of the method to work out what else is going to happen (other than finally blocks).

Solution 2

Now someone told me that this is not very good programming because I use the return statement inside a loop and this would cause garbage collection to malfunction.

That's a bunch of rubbish. Everything inside the method would be cleaned up unless there were other references to it in the class or elsewhere (a reason why encapsulation is important). As a rule of thumb, it's generally better to use one return statement simply because it is easier to figure out where the method will exit.

Personally, I would write:

Boolean retVal = false;
for(int i=0; i<array.length; ++i){
    if(array[i]==valueToFind) {
        retVal = true;
        break; //Break immediately helps if you are looking through a big array
    }
}
return retVal;

Solution 3

There have been methodologies in all languages advocating for use of a single return statement in any function. However impossible it may be in certain code, some people do strive for that, however, it may end up making your code more complex (as in more lines of code), but on the other hand, somewhat easier to follow (as in logic flow).

This will not mess up garbage collection in any way!!

The better way to do it is to set a boolean value, if you want to listen to him.

boolean flag = false;
for(int i=0; i<array.length; ++i){
    if(array[i] == valueToFind) {
        flag = true;
        break;
    }
}
return flag;

Solution 4

Some people argue that a method should have a single point of exit (e.g., only one return). Personally, I think that trying to stick to that rule produces code that's harder to read. In your example, as soon as you find what you were looking for, return it immediately, it's clear and it's efficient.

Quoting the C2 wiki:

The original significance of having a single entry and single exit for a function is that it was part of the original definition of StructuredProgramming as opposed to undisciplined goto SpaghettiCode, and allowed a clean mathematical analysis on that basis.

Now that structured programming has long since won the day, no one particularly cares about that anymore, and the rest of the page is largely about best practices and aesthetics and such, not about mathematical analysis of structured programming constructs.

Solution 5

Since there is no issue with GC. I prefer this.

for(int i=0; i<array.length; ++i){
    if(array[i] == valueToFind)
        return true;
}

View more solutions

Comments

[Poindexter]

What exactly did that person give as a reason for the first not being fine? Messes with the GC?

[Alexander Pavlov]

If GC ever malfunctions, it is the JVM's issue, not your program's... Your code is entirely valid.

[assylias]

"this would cause garbage collection to malfunction": Can you elaborate?

[cheeken]

Some people object to returns inside for loops. And other people object to it, and then make up reasons when asked why ...

[d1e]

This is just a religious war.

[Dave Newton]

Meh; it's generally better to write the most readable code, whatever that means for the number of return statements. For example, if you have a bunch of guard clauses, each (IMO) should return immediately on failure.

[Alexander Pavlov]

As a side note, there is nothing to GC in this method, since there are no object allocations here.

[brain]

A single return statement was part of the code convention where I used to work. I find it leads to lots of nesting and much harder to read code.

[Jon Skeet]

@brain: Indeed. It's usually the result of people taking on an idea without understanding the reason why it's a good idea in a certain context.

[Neil Coffey]

Definitely agree with Dave -- I don't see why "where the method will exit" is of any importance per se. What matters is how easily you can follow the logic of the method.

[Ulrich Eckhardt]

In the presence of exceptions, a single exit is just infeasable, as pretty much any piece of nontrivial code could raise one. It's better to go with the flow of the language here instead of forcing the code to behave according to some rules that might have made sense in a different environment/language.

[Kelly S. French]

@JonSkeet, look what you've done, someone is using this answer to try to work around the Halting Problem.