2

I wrote a code for string reversing using Stack and StringBuilder classes. And I have noticed that 'foreach' loop in this code generates java.util.ConcurrentModificationException, but usual 'for' loop instead does not. So why?

public static String reverse(String str)
{
    Stack<Character> stack = new Stack<>();
    StringBuilder sb = new StringBuilder();

    for (int i = 0; i < str.length(); i++)
        stack.push(str.toCharArray()[i]);
    }


    for (Character c: stack) // generates an exception
    {
        sb.append(stack.pop());
    }

    return sb.toString();
}

I expected a reversed string, but ConcurrentModificationException has occured.

Dmitriy
  • 75
  • 2
  • 8
  • wouldn't this `stack.push(str.toCharArray()[i]);` be like `stack.push(str.charAt(i));` and `while(!st.isEmpty()) { sb.append(stack.pop());}`? – dkb May 31 '19 at 18:02
  • stack.pop() modifies the stack and it’s not allowed to change the collection that is in foreach – Sergiu May 31 '19 at 18:03

4 Answers4

5

You are changing the stack (stack.pop()) while iterating through it. Try using a different loop:

public static String reverse(String str)
{
    Stack<Character> stack = new Stack<>();
    StringBuilder sb = new StringBuilder();

    for (int i = 0; i < str.length(); i++)
        stack.push(str.toCharArray()[i]);
    }


    while(!stack.isEmpty()){
         sb.append(stack.pop());
    }

    return sb.toString();
}
DanielM
  • 3,598
  • 5
  • 37
  • 53
0

ConcurrentModificationExceptions occur when you try and edit something which you are iterating over. By calling stack.pop() you are removing an entity from the entity over which you are currently looping.

I'd recommend instead that you iterate through the stack until it's empty:

while (!stack.isEmpty()) {
    sb.append(stack.pop());
}
A. Bandtock
  • 1,233
  • 8
  • 14
0

The for-each loop uses an iterator in the background which checks if there is a next element in "stack" and then sets the value of "Character c" to that next element. But you are not allowed to change the size of "stack" during the foreach. By calling stack.pop() you are violating that rule. Instead of the for(Characater c : stack) use:

while(!stack.empty()){
   sb.append(stack.pop());
}
PhilKes
  • 78
  • 7
0

pop returns the element at the top of the stack, and then removes that element. You cannot modify a collection while iterating over it with an enhanced for loop, since, as you've seen, you'll get a ConcurrentModificationException.

One approach could be to use a while loop and iterate over the stack until its depleted:

while (!stack.empty()) {
    sb.append(stack.pop());
}
Mureinik
  • 297,002
  • 52
  • 306
  • 350