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.

It depends on the class itself. Guava's ImmutableMap isn't intended to be an immutable view into a mutable class. If your class is immutable and has some structure that is basically an ImmutableMap, then make the return type ImmutableMap. However, if your class is mutable, don't. If you have this:

public ImmutableMap<String, Integer> foo() {
    return ImmutableMap.copyOf(internalMap);
}

Guava will copy the map every time. That's slow. But if internalMap was already an ImmutableMap, then it's totally fine.

If you don't restrict your class to returning ImmutableMap, then you could instead return Collections.unmodifiableMap like so:

public Map<String, Integer> foo() {
    return Collections.unmodifiableMap(internalMap);
}

Note that this is an immutable view into the map. If internalMap changes, so will a cached copy of Collections.unmodifiableMap(internalMap). I still prefer it for getters, however.

This is not answering the exact question, but it is still worth considering whether a map should be returned at all. If the map is immutable, then the primary method to be provided is based on the get(key):

public Integer fooOf(String key) {
    return map.get(key);
}

This makes the API much tighter. If a map is actually required, this could be left up to the client of the API by providing a stream of entries:

public Stream<Map.Entry<String, Integer>> foos() {
    map.entrySet().stream()
}

Then the client can make its own immutable or mutable map as it needs, or add the entries to its own map. If the client needs to know if the value exists, optional can be returned instead:

public Optional<Integer> fooOf(String key) {
    return Optional.ofNullable(map.get(key));
}

Immutable Map is a type of Map. So leaving the return type of Map is okay.

To ensure that the users do not modify the returned object, the documentation of the method can describe the characteristics of the returned object.

This is arguably a matter of opinion, but the better idea here is to use an interface for the map class. This interface doesn't need to explicitly say that it is immutable, but the message will be the same if you don't expose any of the setter methods of the parent class in the interface.

Take a look at the following article:

andy gibson