4

I have the following code:

private List<String> listOfStrings = new ArrayList<>();
listOfStrings.add("a");
listOfStrings.add("b");
listOfStrings.add("c");
listOfStrings.add("d");

for (String temp : listOfStrings) {
  if (temp.equals("c")) {
    Collections.swap(listOfStrings, 0, listOfStrings.indexOf(temp));
  }
}

The list may not just be list of String but could a list of objects defined by the class that I wrote. I'm not sure about the swap here, I see it compiled and running fine but I don't know if it's safe here.

Does anyone have any suggestions on this? If I need to do the swap. I planned to use for (int i = 0; i < size; i++) to iterate and use list.get(i) to get item, but i think it's not good idea to use list.get(i) on an arraylist?

Any help will be appreciated!! Thanks in advance!!

Zip
  • 809
  • 2
  • 14
  • 30

2 Answers2

5

If you are worried about a ConcurrentModificationException, yes, calling swap from within the loop is safe.

The enhanced for loop will use an iterator internally, and an iterator may throw a ConcurrentModificationException when it detects a structural modification of the list which is not done by the iterator itself. Although you do modify the list, you are not doing a structural modification: a structural modification is a modification in which the size of the list (or the backing array) changes. Merely setting a value is not considered a structural modification. From the Java API documentation:

(A structural modification is any operation that adds or deletes one or more elements, or explicitly resizes the backing array; merely setting the value of an element is not a structural modification.)

However, using the index-based for loop in your case will be faster. The reason is that the indexOf(temp) call actually needs to find the object to obtain its index, so it has to loop through the list items again. So your algorithm has quadratic running time. In the index-based for-loop you already know the index of the element you want to swap, so this will not be necessary, and you have linear running time.

Hoopje
  • 12,677
  • 8
  • 34
  • 50
2

I guess your question is 'May this throw a ConcurrentModificationException?'

To answer the question, you should know when and why a ConcurrentModificationException be thrown in a collection.
Take ArrayList for example:

  • ArrayList has a field modCount which means modification count. When invoke some methods which cause structural modification to ArrayList, such as add()/remove(), will increase the modCount``(modCount++).
  • ArrayList.Iterator has a field expectedModCount which is assigned by the ArrayList's modCount when created. When next() or remove() invoked, will compare the expectedModCount with modCount, if not equals, a ConcurrentModificationException will be thrown.

  • Collections.swap's source code:

    public static void swap(List<?> list, int i, int j) {
    final List l = list;
    l.set(i, l.set(j, l.get(i)));
}

    Collections.swap invokes ArrayList's set() method, which does not increase the modCount, so it will not cause the ConcurrentModificationException.

magooup
  • 134
  • 6