CODEWITHSUNDEEP
javaloopssyntax
gavenkoa
gavenkoa

Reputation: 48823

Keep previous element of collection in Java for loop even on continue

I need to match next element with previous. On condition I would like to continue iteration. But I don't like copy/paste assignment to previous element:

List<Type> all;
Type prev = null;
for (Type curr : all) {
    if (prev == null) {
        prev = curr;
        continue;
    }
    if (isBad(curr)) {
        prev = curr;
        continue;
    }
    if (! curr.match(prev)) {
        prev = curr;
        continue;
    }
    process(prev, curr);
    prev = curr;
}

How can I avoid prev = curr duplication?

I think that solution:

for (Type curr : all) {
    if (prev != null && !isBad(curr) && curr.match(prev)) {
        process(prev, curr);
    }
    prev = curr;
}

is cheating because process(prev, curr); shouldn't have indenting. Error checking logic shouldn't bother with main execution logic (forcing of indentation).

Upvotes: 0

Views: 1155

Answers (7)

gavenkoa
gavenkoa

Reputation: 48823

I think about:

List<Type> all;
Type prev = null;
Type curr = null;
Iterator<Type> iter = all.iterator();
for (; iter.haSNext(); prev = curr) {
    curr = iter.next();
    if (prev == null)
        continue;
    if (isBad(curr))
        continue;
    if (! curr.match(prev))
        continue;

    process(prev, curr);
}

I wrote blog post about this topic with more detailed examples.

As a result I find out that Java miss goto statement with which implementation with imposed restrictions would be trivial.

Upvotes: 1

Wietlol
Wietlol

Reputation: 1071

Remove all "continue" from your script.

Either do this by linking the if statements together using "else if" and having process() in an else block at the end, or do so by combining the conditions and swapping the blocks:

for (Type curr : all) {
    if (prev != null &&
        !isBad(curr) &&
        curr.match(prev)) {
        process(prev, curr);
    }
    prev = curr;
}

or

for (Type curr : all) {
    if (prev != null) {
    }
    else if (isBad(curr)) {
    }
    else if (! curr.match(prev)) {
    }
    else {
        process(prev, curr);
    }
    prev = curr;
}

Upvotes: 1

sebi88
sebi88

Reputation: 158

You could use a for loop with indexes, so you can read 'ahead' and you shouldn't use a variable for the previous one, the last value of i have to be size-1 in this case.

    List<Type> all;
    Type prev = null;
    for(int i=0; i<all.size()-1; i++) {
        if(isBad(all.get(i)) {
            continue;
        }
        if(all.get(i).match(all.get(i+1)) {
            continue;
        }

        ...
    }

Upvotes: 2

AR1
AR1

Reputation: 5003

Just use OR (||) and put the condition in a method with relevant name:

public void myMethod(){
    List<Type> all;
    Type prev = null;
    for (Type curr : all) {
            if (isContinue()) {
                prev = curr;
                continue;
            }
            process(prev, curr);
            prev = curr;
    }
}


private boolean isContinue(){
    return (prev == null || isBad(curr) || !curr.match(prev);
}

Upvotes: 1

Sandy
Sandy

Reputation: 66

Why not use OR (||).

i.e.

if (prev == null || isBad(curr) || ! curr.match(prev) ) {
    prev = curr;
    continue;
}

Upvotes: 1

Eran
Eran

Reputation: 393866

You can eliminate the continue statements and make the code clearer by combining the 3 if statements :

for (Type curr : all) {
    if (prev != null && !isBad(curr) && curr.match(prev)) {
        process(prev, curr);
    }
    prev = curr;
}

Upvotes: 1

Tamas Rev
Tamas Rev

Reputation: 7166

As far as I can see, you do the same in all the if-s. So you can use the or || within the if :

for (Type curr : all) {
    if (prev == null || isBad(curr) || ! curr.match(prev)) {
        prev = curr;
        continue;
    }
    process(prev, curr);
    prev = curr;
}

Upvotes: 0

Related Questions