Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

When writing an API in Java, returning an immutable collection of some sort, I've got the option of returning Collection (or List, Map, etc) from the method, or guava's ImmutableCollection (or ImmutableList, ImmutableMap, etc):

private final ImmutableMap<> _immutableMap = ...

public Collection<T> getValues() {
    return _immutableMap.values();
}

or

public ImmutableCollection<T> getValues() {
    return _immutableMap.values();
}

Which is better?

Setting the return value as ImmutableCollection constrains me to use that exact type forever more, so that I can't swap out _immutableMap for my own immutable type later on that returns, say, an immutable view of the values that isnt an ImmutableCollection (constructing one requires copying the values, and I can't subclass it to provide an immutable view of a mutable collection).

But there is benefit to people calling the method to see the return value is actually immutable, and have the add and remove methods marked as deprecated, rather than trying to add or remove items and have it fail at runtime.

Generally, I like to return the most-specific interface type from a method, so that I can change the underlying implementation later on if necessary without breaking contract. But there is benefit to callers returning an ImmutableCollection, at the cost of constraining future development of the method and causing future possible performance problems due to copying all the values into an ImmutableCollection rather than providing an immutable view.

share|improve this question
    
You could wrap it with Collections.unmodifiableCollection. –  Martin Schröder Dec 18 '13 at 11:44

2 Answers 2

up vote 3 down vote accepted

I would normally recommend using the more abstract types (Collection, List etc) in this case.

Reasons:

  • It provides more flexibility to return a different concrete type in the future. Somebody may come up with a better ImmutableList than Guava, for example.
  • Collection, List etc are more familiar to users.
  • It avoids a dependency on Guava in your public API. Avoiding external dependencies is generally good, use the standard APIs wherever you can.

If you are happy to commit to always using Guava collections, or if this is just an internal API that won't be widely used by different people then I guess it doesn't matter so much.

P.S. if your concern is signalling to the user that the return value is immutable, I would suggest finding another way to communicate this (annotations, documentation, method naming etc.). The Java type system isn't really designed for communicating arbitrary facts about types (some more sophisticated type systems attempt this, but it's still a research topic)

share|improve this answer

Generally, I like to return the most-specific interface type from a method

You probably mean the least-specific type, because that's what allows you to exchange the concrete result type by something more specific.

In general, if you want to remain open like this you have to look for the best suited common ancestor of the types you have in mind, then walk all the way up to the root until you think it is no longer suited for your method's return type.

Let's make this example concrete with your two classes:

ImmutableCollection extends Collection extends Iterable extends Object

and

List extends Collection extends Iterable extends Object

Clearly, your common ancestor is Collection, so you must ask yourself the following question:

Would it be ok, if my callers got a Collection back instead of ImmutableCollection?

I guess it would be fine, as they are already assuming a collection anyways. So let's say you write it with Collection<T> as return type. It is easy to see, that you can now simply switch the implementation to an non-modifable java.util.List, without any client code needing a change.

You do indeed lose some information as you pointed out by not knowing from the type, whether your Collection can be modified or not. But, this is a kind of red herring really, because if you open up the Javadoc f.ex. of the add method you can easily read:

Throws:

UnsupportedOperationException - if the add operation is not supported by this collection

Therefore, while one can no longer see from the type itself, that the collection is immutable, you also do not have a type that clearly says it is mutable. In other words, Collection can be read as handle with care either way.

share|improve this answer
    
Unfortunately, if you're returning a (presumably defensively copied) collection then modifying it is perfectly valid. If client code does that then there's no problem. If modifying that collection is what they want to do, then you can't be sure everybody will copy it just in case it becomes immutable in future. Public interface changes like this are breaking, and should be reserved for major version number increments if they're even done at all. –  Phoshi Dec 17 '13 at 14:49
    
Well I agree that the change as such is breaking, but keep in mind that the actual public interface did not change. This is simply a problem of the limited type system not being able to distinguish modifiable and unmodifiable lists (in contrast to s.th. like Scala, where such a change would also require an interface change). All I'm saying is that if you add to a collection and it no longer works, you are no more in a position to blame the interface than when you expect a List to give you constant-time random access. –  Frank Dec 17 '13 at 15:03
    
While I agree it's a failure of the type system (Seriously, Java? You have methods documented to be fallible? Nice job on LSP, there.) if you're returning a collection and it behaves in some way people are gonna use it for that. Documentation is no replacement for cold, hard, compile-time assurances and some people just ain't gonna read your docs, making it unfortunately a breaking change. Consistently imperfect > inconsistently perfect in cases like these, IMO. –  Phoshi Dec 17 '13 at 15:12
    
At least for in-house usage tests should save you from breaking things in production.. if the API is used outside, well you're gonna have to die one death, as I'm afraid there's no silver bullet. –  Frank Dec 17 '13 at 15:29
    
I don't see there being a huge issue with returning a defensively copied mutable array. It doesn't get the point across as well as an immutable one would, but your API is still valid and strong. If you're making other breaking changes too, then go for it, but standalone this seems like one of those times when "the perfect is the enemy of the good". –  Phoshi Dec 17 '13 at 15:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.