Is it better to return an ImmutableMap or a Map?

Let's say I'm writing a method that should return a Map. For instance:

public Map<String, Integer> foo() {
  return new HashMap<String, Integer>();
}

After thinking about it for a while, I've decided that there is no reason to modify this Map once it is created. Thus, I would like to return an ImmutableMap.

public Map<String, Integer> foo() {
  return ImmutableMap.of();
}

Should I leave the return type as a generic Map, or should I specify that I'm returning an ImmutableMap ?

From one side, this is exactly why interfaces were created for; to hide the implementation details.
On the other hand, if I'll leave it like this, other developers might miss the fact that this object is immutable. Thus, I won't achieve a major goal of immutable objects; to make the code more clear by minimizing the number of objects that can change. Even worst, after a while, someone might try to change this object, and this will result in a runtime error (The compiler will not warn about it).

• If you are writing a public-facing API and that immutability is an important aspect of your design, I would definitely make it explicit either by having the name of the method clearly denotes that the returned map will be immutable or by returning the concrete type of the map. Mentioning it in the javadoc is not enough in my opinion.

  Since you're apparently using the Guava implementation, I looked at the doc and it's an abstract class so it does give you a bit of flexibility on the actual, concrete type.

• If you are writing an internal tool/library, it becomes much more acceptable to just return a plain Map. People will know about the internals of the code they are calling or at least will have easy access to it.

My conclusion would be that explicit is good, don't leave things to chance.

You should have ImmutableMap as your return type. Map contains methods that are not supported by the implementation of ImmutableMap (e.g. put) and are marked @deprecated in ImmutableMap.

Using deprecated methods will result in a compiler warning & most IDEs will warn when people attempt to use the deprecated methods.

This advanced warning is preferable to having runtime exceptions as your first hint that something is wrong.

On the other hand, if I'll leave it like this, other developers might miss the fact that this object is immutable.

You should mention that in the javadocs. Developers do read them, you know.

Thus, I won't achieve a major goal of immutable objects; to make the code more clear by minimizing the number of objects that can change. Even worst, after a while, someone might try to change this object, and this will result in a runtime error (The compiler will not warn about it).

No developer publishes his code untested. And when he does test it, he gets an Exception thrown where he not only sees the reason but also the file and line where he tried to write to an immutable map.

Do note though, only the Map itself will be immutable, not the objects it contains.

if I'll leave it like this, other developers might miss the fact that this object is immutable

That's true, but other developers should test their code and ensure that it is covered.

Nevertheless you have 2 more options to solve this:

• Use Javadoc

  @return a immutable map
  

• Chose a descriptive method name

  public Map<String, Integer> getImmutableMap()
  public Map<String, Integer> getUnmodifiableEntries()
  

  For a concrete use case you can even name the methods better. E.g.

  public Map<String, Integer> getUnmodifiableCountByWords()
  

What else can you do?!

You can return a

• copy

  private Map<String, Integer> myMap;
  
  public Map<String, Integer> foo() {
    return new HashMap<String, Integer>(myMap);
  }
  

  This approach should be used if you expect that a lot of clients will modify the map and as long as the map only contains a few entries.

• CopyOnWriteMap

  copy on write collections are usually used when you have to deal with
  concurrency. But the concept will also help you in your situation, since a CopyOnWriteMap creates a copy of the internal data structure on a mutative operation (e.g. add, remove).

  In this case you need a thin wrapper around your map that delegates all method invocations to the underlying map, except the mutative operations. If a mutative operation is invoked it creates a copy of the underlying map and all further invocations will be delegated to this copy.

  This approach should be used if you expect that some clients will modify the map.

  Sadly java does not have such a CopyOnWriteMap. But you might find a third party or implement it by yourself.

At last you should keep in mind that the elements in your map might still be mutable.

Definitely return an ImmutableMap, justification being:

• The method signature (including return type) should be self-documenting. Comments are like customer service: if your clients need to rely on them, then your primary product is defective.
• Whether something is an interface or a class is only relevant when extending or implementing it. Given an instance (object), 99% of the time client code will not know or care whether something is an interface or a class. I assumed at first that ImmutableMap was an interface. Only after I clicked the link did I realize it is a class.