Should defensive copies ALWAYS be made?

Question

I have a code which takes a bipartite graph as an input and returns a map with key"1" with value which is a list of "nodes in set1" and key"2" whose value is list of "nodes in set2". Now, map is mutable. In theory I should use a defensive copy for returning a map. But, in this case is it really required ? It appears an overkill.

ex:

class BiPartite {

   Graph graph;
   Map bipartite

   Bipartite(graph) {
      this.graph = graph;
   }

   void calcBipartite() {
     // calculate map 
   }

   Map getMap() {
     // should i make defensive copy ? Appears overkill. 
   }  
}

Answer 1

It depends :)

• You can document your class accordingly, specifying that the returned Map is a mutable direct view on the mutable state of the BiPartite object. I wouldn't recommend that however.
• You can wrap the internal Map with Collections.unmodifiableMap(map) in getMap() and document that it reflects the mutable state of the BiPartite object. That can be a valid approach as long as the BiPartite object is not supposed to be threadsafe. If the client wants to keep the returned Map as a stable snaphot, she can copy it herself. If that's not needed, she can benefit from the fast wrapping operation.
• You can always return a full copy. That'd make most sense if you actually make the BiPartite object threadsafe. In that case you'll also have to synchronize all operations of the internal map (including the map copying operation!).

Basically it boils down to: Think about how the BiPartite class and its methods should be used, pick a suitable implementation for that and clearly document both the behavior of the class and the reasoning behind it.