6

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.

2

2 Answers 2

5

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)

2
  • 1
    3 years late, but I find the P.S. section is helpful. I've been oblivious to this and tried to commit this particular "sin" a number of times by now. Do you have any good pointer on the "sophisticated type systems" and the research topic?
    – tkokasih
    Aug 17, 2016 at 2:39
  • @tkokasih One example (both as a usable tool and a developing research program) is the checker framework. You could use the papers from that group as a jumping off point, or perhaps more generally search for research on static analysis, type inference, and immutability. Apr 23, 2021 at 15:03
1

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.

19
  • 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, 2013 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, 2013 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, 2013 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, 2013 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, 2013 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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