Modifying each item of a List in java

Question

I'm just starting to work with lists in java. I'm wondering what the recommended method to modify each element of a list would be?

I've been able to get it done with both the following methods, but they both seem fairly unelegant. Is there any better way to get this done in java? And is any of the below methods recommended over the other, or are both on the same level?

//Modifying with foreach
for (String each : list)
{
    list.set(list.indexOf(each), each+ " blah");
}

//Modifying with for
for (ListIterator<String> i = list.listIterator(); i.hasNext(); i.next()) 
{
    i.next();
    list.set(i.nextIndex()-1, i.previous() + " blah yadda");
}

Answer 1

The second version would be better. Internally they are the same in the end, but the second actually allows you to modify the list, while the first one will throw a ConcurrentModificationException.

But then you are using the Iterator in a wrong way. Here is how you do it correctly:

for (final ListIterator<String> i = list.listIterator(); i.hasNext();) {
  final String element = i.next();
  i.set(element + "yaddayadda");
}

The iterator is the one that needs to modify the list as it is the only one that knows how to do that properly without getting confused about the list elements and order.

Edit: Because I see this in all comments and the other answers:

Why you should not use list.get, list.set and list.size in a loop

There are many collections in the Java collections framework, each on optimized for specific needs. Many people use the ArrayList, which internally uses an array. This is fine as long as the amount of elements does not change much over time and has the special benefit that get, set and size are constant time operations on this specific type of list.

There are however other list types, where this is not true. For example if you have a list that constantly grows and/or shrinks, it is much better to use a LinkedList, because in contrast to the ArrayList add(element) is a constant time operation, but add(index, element), get(index) and remove(index) are not!

To get the position of the specific index, the list needs to be traversed from the first/last till the specific element is found. So if you do that in a loop, this is equal to the following pseudo-code:

for (int index = 0; index < list.size(); ++index) {
  Element e = get( (for(int i = 0; i < size; ++i) { if (i == index) return element; else element = nextElement(); }) );
}

The Iterator is an abstract way to traverse a list and therefore it can ensure that the traversal is done in an optimal way for each list. Test show that there is little time difference between using an iterator and get(i) for an ArrayList, but a huge time difference (in favor for the iterator) on a LinkedList.

Answer 2

EDIT: If you know that size(), get(index) and set(index, value) are all constant time operations for the operations you're using (e.g. for ArrayList), I would personally just skip the iterators in this case:

for (int i = 0; i < list.size(); i++) {
    list.set(i, list.get(i) + " blah");
}

Your first approach is inefficient and potentially incorrect (as indexOf may return the wrong value - it will return the first match). Your second approach is very confusing - the fact that you call next() twice and previous once makes it hard to understand in my view.

Any approach using List.set(index, value) will be inefficient for a list which doesn't have constant time indexed write access, of course. As TwoThe noted, using ListIterator.set(value) is much better. TwoThe's approach of using a ListIterator is a better general purpose approach.

That said, another alternative in many cases would be to change your design to project one list to another instead - either as a view or materially. When you're not changing the list, you don't need to worry about it.