Better Way to iterate through those ArrayLists?

Question

I have the following Code:

/**
 * Calculates all correct Moves for Figure f which are possible
 * @param f the Figure for which you want to check it's moves
 * @return
 */
public ArrayList<Move> checkMoves(Figure f) {
    ArrayList<Move> moves = this.expand(f.getLocation(), new ArrayList<Move>(), false, this.getPlayer(f.getOwner()));

    // Fix the wrongly calculated moves
    for(Move m : moves) {
        moves = this.jumpFix(moves, m);
    }

    return moves;
}

/**
 * Takes a look into all calculated moves and finds those which should be seen as one move, but are still
 * considered to be more than one move (several jumps in one move)
 */
private ArrayList<Move> jumpFix(ArrayList<Move> moves, Move m) {
    ArrayList<Move> consecutive = this.findConsecutiveMoves(moves, new ArrayList<Move>(), m);

    if(consecutive.size() > 0) {
        m.getTarget().setX(consecutive.get(consecutive.size() - 1).getTarget().getX());
        m.getTarget().setY(consecutive.get(consecutive.size() - 1).getTarget().getY());
    }

    for(Move i : consecutive) {
        moves.remove(i);
        m.addStop(i.getTarget());
    }

    return moves;
}

/**
 * Finds all consecutive moves to the given move m, that share their start and target cells
 */
public ArrayList<Move> findConsecutiveMoves(ArrayList<Move> moves, ArrayList<Move> deleteable, Move m) {
    for(Move n : moves) {
        if(n.getStart().equalTo(m.getTarget())) {
            deleteable.add(n);
            deleteable = this.findConsecutiveMoves(moves, deleteable, n);
        }
    }

    return deleteable;
}

Explanation: - checkMoves calculates all possible Moves for given Figure f - After expand() calculated all possible moves, several of those "moves" might be sondered one move, if they are connected (Example: A Move is going from a to b. If we now have several Moves a -> b, b -> c, c -> d this would be the Move a -> d) - I need to remove all those little consecutive moves and set a -> b to a -> d - I am trying to iterate through the whole moves List and start findConsecutiveMoves for each m, checking if there are consecutive moves.

I do get a StackOverflowError. I suppose my way of doing things is not good. How can i improve it?
I fear I will get a java.util.ConcurrentModificationException in case the program finds consecutive moves, since I'm iterating over moves while changing it. How to avoid that?

score 0 · Answer 1 · answered May 20 '14 at 12:32

0

Put in a list the index of removed items and iterate again on the list with these index to add the new item.

answered May 20 '14 at 12:32

Samy Sup

298
2
16

score 0 · Answer 2 · answered May 20 '14 at 12:35

0

The stackoverflow error occurs due to recursive method calls, you must check your logic for any recursive method calls, to avoid the

java.util.ConcurrentModificationException

You should use the Collections.synchronizedCollection(Collection c) method which will provide you a thread safe collection, One more thing about iterating, if you are using JAVA 8 then you should checkout the streams API which provides better and efficient way for iterating.

answered May 20 '14 at 12:35

Bilbo Baggins

2,899
10
52
77

@ I checked my findConsecutiveMoves() function several times. I cannot find any flaws in it's logic. There just has to be a better way to find all consecutive ones? – May 20 '14 at 12:37
1

Here is better explanation http://stackoverflow.com/questions/10073471/java-how-to-avoid-stackoverflowexception – Bilbo Baggins May 20 '14 at 12:39
One more, read the answer of Derek Elkins in this question http://stackoverflow.com/questions/951635/how-to-handle-stackoverflowerror-in-java – Bilbo Baggins May 20 '14 at 12:41

score 0 · Answer 3 · answered May 20 '14 at 12:38

StackOverflowError means you have to many recursive calls. Probably your findConsecutiveMoves goes into an infinite loop (checking nightbours in a infinite loop). To solve this, mark your Moves you have already checked and only check next Moves which are not marked yet.

To prevent ConcurrentModificationException Add a list, where you store deleteable Moves. After the whole iteration is finshed you can delete every Move that is in this list, while you are not iteration of your Moves list.

Better Way to iterate through those ArrayLists?

3 Answers3