A method declaring a mutable data structure as an output and returning an immutable one actually

Question

Lately, I'm having a heated discussion regarding this issue.

Lets say I created this method in Java:

 public Set<String> getRich() {
    return ImmutableSet<String> ....;
 }

Whenever I see that in a pull request, I shout and try to explain why it is wrong. By doing that, I'm misguiding the consumers of my method by the promise that they will get a Set. This means they can remove or add elements. javac will happily compile that but RuntimeException will be thrown. Add to that, it is violating "Liskov Substitution Principle".

Personally, I always do:

public ImmutableSet<String> getRich() {
    return ImmutableSet<String> ....;
}

This way, no one is going to shot himself in the foot.

One suggested approach is return Iterable. I think this is bad, because the consumer of that method will lose the potential power of HashSet, HashMap or whatever (by not be able to call set.get(hash) ).

Another approach is - as a consumer - to create a copy of the output of getRich() method. But how are you going to be sure that the consumers will do that?

And of course, you have the folks who will stick to OOP design principles and say: "Always program to interfaces". To me - in this particular case - this is the worst option ever.

How would you deal with this case?

(Off topic: This case is a perfect example how static typing might ensure correctness but it will never ensure rightfulness).

Answer 1

Technically, declaring the return type as Set<T> is not a violation of the LSP.

The documentation for Set<T>.Add explicitly states that implementations might throw an UnsupportedOperationException. Which means it's the clients responsibilities to check whether the set is mutable.

Now, whether making ImmutableSet<T> extend Set<T> was a good choice, that's debatable. I personally think it was a very bad choice - client code should not be littered with try/catches to find out whether a set is mutable. Furthermore, Set<T> does not even provide an isMutable() method to let clients perform the check easily! Having a separate interface for immutable collections would be a much cleaner design.

I personally would work around this design flaw and, as you suggested, declare the return type as ImmutableSet<T>.

The second part of your question is whether you should return ImmutableSet<T> or Iterable<T>: that really depends on what you think clients will expect from your API. Things to consider:

• Is your implementation likely to change? Do you think you'll ever need to change the business logic and return some other collection? If so, go with Iterable<T>;
• Do your clients explicitly expect an ImmutableSet<T>? If not, go with Iterable<T>;
• Otherwise, you might want to adhere to the Robustness principle, which states "Be contravariant in the input type and covariant in the output type." In other words, choose the most abstract type possible for your arguments types, and the most specific type possible for your return types.