What is the fastest way to get the elements of a collection?

Question

I have a List<Pair<String, String>> into which I want to copy the data from a Collection.

What is the best way to read the collection and add items to list?

List<Pair<String, String>> identityMemoPairs = new LinkedList<Pair<String, String>>();
Collection result = handler.getResult();

    while(result.iterator().hasNext()){
         IdentityM im =(IdentityM) result.iterator().next();
         identityMemoPairs.add(Pair.of(im.identity,im.memo));
    }

Well, I'd say the best way is ... read the collection and add the transformed values to the list. There's not much more to that. :) - Seriously, what's the problem? — Thomas, Feb 01 '12 at 15:47
Check http://stackoverflow.com/questions/322715/when-to-use-linkedlist-over-arraylist to learn about performance of List implementations. When iterating, there is no difference. — jalopaba, Feb 01 '12 at 15:54
Did the answers help you solve your problem? If yes, please select one as the accepted answer. If no, please provide additional information on what is still the problem. — Philipp Wendler, Feb 21 '12 at 17:09

Philipp Wendler · Accepted Answer · 2012-02-01T16:51:22.593

Your code is wrong, because you create a new iterator at each iteration of the while loop (actually you create two of them). Each fresh iterator will point to the beginning of the result collection. Thus you have created an infinite loop.

To fix this, call result.iterator() only once and store the result in a variable.

But even better (better to read, less error-prone) would be the for-each loop, which is (almost always) the preferred variant to iterate over collections:

for (IdentityM im : (Collection<IdentityM>)result) {
  identityMemoPairs.add(Pair.of(im.identity,im.memo));
}

The compiler will automatically transform this into code using the iterator, so no performance difference. Generally, performance is anyway not a problem when iterating over a collection, as long as you avoid a few bad things like calling get(i) on a LinkedList.

Note that the compiler will give you a warning here, which has nothing to do with the iteration, but with the use of the raw type Collection (instead of Collection<IdentityM>). Please check if handler.getResult() actually returns a Collection<IdentityM> and change the type of the result variable to this if possible.

Another question is whether you really need that list to be a list of pairs. Usually using plain pair classes is not recommended, because their name does not show the meaning of the object they represent. For example, it is better to use a class PersonName which has fields for first and last name instead of a Pair<String, String>. Why can't you just use a List<IdentityM>? And if you can use this, are you sure you can't use a Collection<IdentityM>? (List and Collection are often exchangeable.) Then you could avoid the copying altogether.

score 0 · Answer 2 · answered Feb 01 '12 at 15:50

Your code is imho good as it is. But you could probabbly make the handler return a collection of Pairs directly so you could call identityMemoPairs.addAll() instead of iterating the collection yourself. But that only makes it "prettier" it doesn't give more performance.

What is the fastest way to get the elements of a collection?

2 Answers2