views:

195

answers:

7

What's the best way to return a collection in Java?

Should I allow the caller to provide a collection to add to? Or just return a List<> or Set<> of items? Or both?

public class Item { ... }

public class SomeOtherClass
{
  private List<Item> myItems;

  public List<Item> getItems()
  { 
     return Collections.unmodifiableList(this.myItems); 
  }
  public void collectItems(Collection<? super Item> target)
  {
     target.addAll(myItems);
  }
}

note: the above example assumes the pre-existence of a list that can be instantly returned. I am also interested in the appropriate answer when such a list does not previously exist and must be generated when the caller calls getItems() or collectItems(). (I have renamed collectItems based on the point raised by Mykola.)

+11  A: 

It is better (unless some performance issues) to return result in a functions via return. In that way it is more clear what is going on.

If you choose second option (fill client's collection) than it would be better to rename function from getItems to something like fillWithItems to avoid ambiguous code.

Also don't forget about JavaBeans and its convention.

Mykola Golubyev
+1 for suggesting a rename.
Jason S
A: 

You should return a the collection. It is a more common approach in Java than to use in/out parameters. I don't see any reason there would be a performance penalty for returning a large collection and it will be much cleaner code.

Casey
Penalties are for making collection that you return to be immutable.
Mykola Golubyev
So, return a copy or clone of the collection. No less of a penalty than copying the collection into a user created one.
Casey
A: 

Given the way Java works you would generally expect the return version.

However if you need control over what type of collection is created then you would do the version where you pass it in as an argument.

Usually nothing should care what type of collection is created, so you should generally go with the return version. Good use of the unmodifiableList by the way.

TofuBeer
+6  A: 

I'd just prefer the List<Item> getItems() method. There's no real advantage to void getItems(Collection<? super Item> target) over the caller just doing myCollection.addAll(foo.getItems()) performance or otherwise. Collections.unmodifiableXYZ only creates a wrapper, not a complete copy of the collection so if the wrapper is used immediately and discarded it will never make it out of the first generation and will be collected quickly with little overhead.

If the collection of items is not always realized you might consider having getItems return Iterable<Item> when you don't know how many items there are. If you know the number of items and can write an iterator for them, then it's easy to write a custom subclass of AbstractCollection and return that.

Geoff Reedy
+1 for mentioning iterators
Jason S
A: 

Note: Returning a Set and returning a List have different implications.

A Set has no duplicates and no stated order. Adding an element to a set might result in a different order of the elements.

A List may contain duplicates and adding an element will not (generally) change the overall order of the list.

As for how you return the list, I would use the first form:

public List<Item> getItems()
{ 
   return Collections.unmodifiableList(this.myItems); 
}

I can't think of a situation where the latter form would be of any benefit. A List is not like an array where the space can be pre-allocated. So, there's no performance savings by passing in a List.

Devon_C_Miller
If I am gathering several collections together, and each of them could have 10,000 items, I would rather pass in a destination collection in its place. Or if the type of the collection matters (set vs. list) and there are large #s of items.
Jason S
A: 

The only reason I can think of to fill an existing collection rather than making a new one is when you have issues with the type of object in the collection. Like the Java library toArray(Object[] a) function, where the program doesn't know at compile time what the appropriate type of the elements of the array will be, so it can't just return, e.g. a String[]. So instead they have the caller pass in an array with the appropriate type of elements, and they fill that.

90% of the time you know exactly what types of objects you want to return, so you can just do it.

Jay
A: 

You could change your signature to return a Collection or Iterable. For returning Iterable, you could return a new something-Iterable(myItems.iterator()) instead of myItems directly to avoid the client possibly trying to cast to a List (and modify it). If you don't want them modifying the List, also consider returning a Iterator also, but note that Iterable is better since you can directly use those in for-each loops.

Returning an Iterable both makes your intention clear and in the example above, prevents modification. The only implication is that you have lost random access, which may or may not be a problem for your needs.

GreenieMeanie
Given that Iterable is an interface, `new Iterable(myItems.iterator())` is not legal; did you mean something else?
Software Monkey
Yeah whoops, you would have to return a new anonymous class (or default Iterable) that implements it.
GreenieMeanie