Atom Feed SITE FEED   ADD TO GOOGLE READER

Be Minty #1: returning Collections

In this series I document patterns, techniques and styles for designing fantastic APIs in Java. My target audience is software developers working with big teams on big projects. Many of the suggestions should be overkill for a weekend project. Topics in this series will range from simple style issues to broad application design strategies.

Aside from my packaging, nothing is original - I take inspiration from Effective Java, Kevin B, Zorzella Z, and the rest of my fine coworkers.

My first Minty API initiates and tracks deliveries. Let's look at one interface method in detail:
  /**
* Returns all deliveries that have started but not yet completed.
*
* @return a possibly-empty List.
*/
public List<Delivery> getDeliveriesInProgress(long customerId);

Ordering must be specified


This API does not specify the ordering of the returned list. This is a problem. Suppose a caller takes the returned list and immediately displays it in a table on screen - a reasonable thing to do. Now the user interface is dependent on a subtle implementation detail of the method - the ordering of the deliveries. If the method just-so-happens to return deliveries ordered by ID, and the IDs just-so-happen to be assigned chronologically, then the UI will probably look great! The deliveries will be oldest to newest.

But now reasonable changes to the implementation - such as how IDs are assigned, or how the list is ordered - break the UI. An awkward UI isn't the only possible problem here. Other calling code might (incorrectly) assume that the most recent delivery is always last in the list, so business logic could break if the ordering changes.

The fix is simple but requires prudency - always specify the ordering of a returned collection in Javadoc:
  /**
* Returns all deliveries that have started but not yet completed.
*
* @return a possibly-empty List, ordered by delivery-requested time.
*/
public List<Delivery> getDeliveriesInProgress(long customerId);

And of course, your implementations need to provide this behaviour.

Returned values must be immutable


By not trusting the caller of your API, you can prevent them from introducing subtle bugs. Consider this calling code:
    List<Delivery> deliveries = deliveryService.getDeliveriesInProgress(customerId);

/* don't show expedited deliveries on this screen */
for (Iterator<Delivery> d = deliveries.iterator(); d.hasNext(); ) {
Delivery delivery = d.next();
if (delivery.isExpedited()) {
d.remove();
}
}
showDeliveries("Standard shipping", deliveries);

This caller is directly mutating the returned value from our method. Returning mutable types allows for subtle bugs. If the implementation doesn't always return a newly-allocated list, callers could get results without expedited deliveries.

Instead, returned collections should always be immutable. Google Collections makes this easy with the immutableList() method.

Wrapping Up


Big applications are hard because bugs live in the details. If you return collections often, these patterns will save bugs.
The problem with immutableList is that it returns List, which has methods remove, add, etc. which cause the user's code to blow up; they don't have any way of knowing that they've been given an immutable list.

Back when Sun designed collections it's too bad they didn't start the top of the hierarchy with ImmutableIterable, whose iterator method returns an ImmutableIterator, then subclass ImmutableIterable with Iterable, which is read/write. ImmutableIterator would not have remove but would have hasNext and next.
See the Java Collections Design Faq.
Thanks for that link.
I don't think returned collections must be immutable, I think it's important to document mutability. Or at least, mutability may be the exception.
Modifiable - anyone can change it
Mutable - someone can change it

Returning modifiable types is bad. It prevents the method implementor from caching the returned value. If you return modifiable types, you need to return a new copy each time.

Returning mutable types is also bad. Since the caller won't receive callbacks if the returned collection changes, such changes could crash the caller. Consider a caller that manually copies a collection into a fixed-size array.