enhanced for with remove runs more efficiently

Question

The following code is itself in an outer loop where oldChar is defined:

List<Character> modifiableCollection = new ArrayList<Character>(Arrays.asList(AlphabeticalStatistics.ALL_LETTERS));
for (int j = 0; j < modifiableCollection.size(); j++) {
    Character letterOfAlphabet = modifiableCollection.get(j);
    String loaAsString = letterOfAlphabet.toString();
    if (replaceLetters(Character.toString(oldChar), loaAsString)) {
        modifiableCollection.remove(j);
        System.out.println(oldChar + " was replaced with " + loaAsString);
        break;
    }
}

I am trying to iterate through ALL_LETTERS as efficiently as possible. For each element, I want to check if replaceLetters() on that letter and another letter. If so, the letter will be replaced and I want to remove this from the modifiableCollection so the next time this is looped it won't look to replace that letter again.

However, what I am seeing is using an enhanced for loop WITH REMOVAL, the code runs AND is more efficient. I must not remove in an enhanced for:

 for (Character letterOfAlphabet : modifiableCollection) {...remove()} // Compiles 

, but when I use a regular for loop, like above, the program takes longer to run; when I use iterator, the program takes even longer to run. Should I stick to the foreach loop?

Edit replaceLetters() method:

protected boolean replaceLetters(String toReplace, String replacement, String toAdd, String toAdd2) {
        if (replacedLetters.contains(toAdd2) || solvedLetters.contains(toAdd)) 
            return false;
        return isCorrect(toReplace, replacement, toAdd, toAdd2);
    }

private boolean isCorrect(String toReplace, String replacement, String toAdd, String toAdd2) {
    String newText = getText().replace(toReplace,  replacement);
    if (Arrays.stream(newText.split(" "))
            .filter(w -> {
                return w.contains(replacement) && AlphabeticalStatistics.needsNoLetters(w);
            })
            .noneMatch(w -> {
                return !EnglishDeterminer.isWord(w);
            })
            ) {
        setText(newText);
        solvedLetters.add(toAdd);
        replacedLetters.add(toAdd2);
        return true;
    }
    return false;
}

The purpose of this program is to decipher a substitution ciphertext.

Answer 1

I seems faster with the indexed for loop (for int i=0; i<size; i++). Removal is probably faster by index as well since you dont have to compare your collection values to find the match. Maybe you should extract a variable for modifiableCollection.size() since it will be evaluated each iteration.

 public static final int ITERATIONS = 100000000;

 public static void main(String[] args) {
    List<Character> alphabetList = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmopqrstuvwxyz123456789".chars()
            .mapToObj(c -> Character.valueOf((char) c)).collect(Collectors.toList());
    ArrayList<Character> copy = new ArrayList<>(alphabetList);
    List<Character> unmodifiableAlphabetList = Collections.unmodifiableList(copy);

    double timeA = checkTime(ITERATIONS, () -> {
        List<Character> modifiableCollection = new ArrayList<>(alphabetList);
        modifiableCollection.removeIf(next -> checkRemove(next));
    });
    System.out.println("A : " + timeA);

    double timeB = checkTime(ITERATIONS, () -> {
        List<Character> modifiableCollection = new ArrayList<>(alphabetList);
        Iterator<Character> iterator = modifiableCollection.iterator();
        while (iterator.hasNext()) {
            if (checkRemove(iterator.next())) {
                iterator.remove();
                break;
            }
        }
    });
    System.out.println("B : " + timeB);

    double timeC = checkTime(ITERATIONS, () -> {
        List<Character> modifiableCollection = new ArrayList<>(alphabetList);
        int size = modifiableCollection.size();
        for (int i = 0; i < size; i++) {
            Character character = unmodifiableAlphabetList.get(i);
            if (checkRemove(character)) {
                modifiableCollection.remove(i);
                break;
            }
        }
    });
    System.out.println("C : " + timeC);

    double timeD = checkTime(ITERATIONS, () -> {
        List<Character> modifiableCollection = new ArrayList<>(alphabetList);
        for (Character c : unmodifiableAlphabetList) {
            if (checkRemove(c)) {
                modifiableCollection.remove(c);
                break;
            }
        }
    });
    System.out.println("D : " + timeD);
}

private static boolean checkRemove(Character next) {
    return next.equals('W');
}

private static double checkTime(long count, Runnable fn) {
    List<Long> diffs = new ArrayList<>();

    for (int i = 0; i < count; i++) {
        long now = System.nanoTime();
        fn.run();
        long after = System.nanoTime();
        long nanoDiff = after - now;
        diffs.add(nanoDiff);
    }
    double average = diffs.stream()
            .mapToLong(l -> l)
            .average()
            .orElse(0L);
    return average;
}

A : 247.68729885
B : 83.9981085
C : 63.98897325
D : 91.69348503