Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it safe to do a Collections.swap() inside a arraylist for loop?

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!!

like image 355
Zip Avatar asked Mar 12 '23 12:03

Zip


1 Answers

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.

like image 153
Hoopje Avatar answered Apr 07 '23 04:04

Hoopje