4
\$\begingroup\$

I much prefer the Java enhanced for each loop, but I often find there are certain times where I need to break from a loop, take the following contrived example:

final List<String> strings = new ArrayList<>(
    Arrays.asList(new String[] { "test", "new", "break", "notThere","last" }));

boolean found;
for(String s : strings) {
  found = s.equals("break");
  if (found) { break; }
}

Needs to be rewritten as the following to avoid the break statement:

int i;
boolean found;
for (found = false, i = 0; i < strings.size() && !found; i++) {
  found = strings.get(i).equals("break");
}

Which would you prefer to come across? I very much prefer the second format (in the case where I need to break), but I have a feeling I'm in the minority.

Edit: I do need to keep track of where the item was located.

\$\endgroup\$
  • \$\begingroup\$ May I ask what's wrong with just using the break statement? \$\endgroup\$ – fredoverflow Jun 7 '12 at 10:40
  • \$\begingroup\$ Anything more then a for(String s : strings) {} or for(int i = 0; i < strings.size(); i++) {} is not something I usually find in code, and it takes a longer time to understand what it is doing, and what it's conditions are. \$\endgroup\$ – JMC Jun 8 '12 at 17:09
5
\$\begingroup\$

I'd use List.indexOf(Object o). From the documentation:

Returns the index of the first occurrence of the specified element in this list, or -1 if this list does not contain the element. More formally, returns the lowest index i such that (o==null ? get(i)==null : o.equals(get(i))), or -1 if there is no such index.

If you need a more complex examination you still can extract out the loop to a method with multiple return statements:

public int getFirstIndexOfSomething(final List<String> input) {
    int i = 0;        
    for (String s: input) {
        if (... check s here ...) {
            return i;
        }
        i++;
    }
    return -1;
}

If you want a really bulletproof solution you can use a custom SearchResult object to store the result and its index (if there is any result):

public class SearchResult {
    private final boolean found;
    private final int index;

    private SearchResult(final boolean found) {
        this.found = false;
        this.index = -1;
    }

    private SearchResult(final int index) {
        if (index < 0) {
            throw new IllegalArgumentException("index cannot be less than zero");
        }
        found = true;
        this.index = index;
    }

    public static SearchResult createFound(final int index) {
        return new SearchResult(index);
    }

    public static SearchResult createNotFound() {
        return new SearchResult(false);
    }

    public boolean isFound() {
        return found;
    }

    public int getIndex() {
        if (!found) {
            throw new IllegalStateException();
        }
        return index;
    }
}

Usage:

public SearchResult getFirstIndexOfSomething(final List<String> input) {
    int i = 0;        
    for (String s: input) {
        if (... check is here ...) {
            return SearchResult.createFound(i);
        }
        i++;
    }
    return SearchResult.createNotFound();
}

It does not let client codes to get invalid indexes since the getIndex method throws an exception when the for loop do not find any result.

A more sophisticated solution is using polymorphism to separate the two states to two simpler classes with a common interface:

public interface SearchResult {

    boolean isFound();

    int getIndex();
}

public class NoResult implements SearchResult {

    public NoResult() {
    }

    @Override
    public boolean isFound() {
        return false;
    }

    @Override
    public int getIndex() {
        throw new IllegalStateException();
    }
}

public class Result implements SearchResult {

    private final int index;

    public Result(final int index) {
        if (index < 0) {
            throw new IllegalArgumentException("index cannot be less than zero");
        }
        this.index = index;
    }

    @Override
    public boolean isFound() {
        return true;
    }

    @Override
    public int getIndex() {
        return index;
    }
}

Usage:

public SearchResult getFirstIndexOfSomething(final List<String> input) {
    int i = 0;        
    for (String s: input) {
        if (... check is here ...) {
            return new Result(i);
        }
        i++;
    }
    return new NoResult();
}
\$\endgroup\$
4
\$\begingroup\$

I suppose you feel that foreach is a kind of a map, and that inhibits you from exiting in the middle. In most code that I have seen, the first form is preferred except in the cases where we need to get the index where the break happened.

i.e This is ugly.

int found = 0;
for(String s : strings) {
  if (s.equals("break")) break;
  found++;
}

I myself prefer the first form because it avoids the need to declare an extra variable, and in my (highly subjective) opinion, extra variables tend to clutter up program, and inhibits comprehension.

In your updated example, I will jump in your boat if you change the for statement to while and remove found :).

\$\endgroup\$
  • \$\begingroup\$ My contrived example forgot to mention that I need access to where I located the item. Adding that. \$\endgroup\$ – Scott Jun 1 '12 at 20:39
  • 1
    \$\begingroup\$ Modesty is a nice trait but your preference here is distinctly non-subjective. Less variables do reduce the complexity of the code, that’s an objective advantage. \$\endgroup\$ – Konrad Rudolph Jun 8 '12 at 9:30
1
\$\begingroup\$

I would use

    if (list.contains("break")) { 
        ...
    }

If inavoidable, the enhanced one I personally find more readable

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.