This week's book giveaways are in the Refactoring and Agile forums.
We're giving away four copies each of Re-engineering Legacy Software and Docker in Action and have the authors on-line!
See this thread and this one for details.
Win a copy of Re-engineering Legacy Software this week in the Refactoring forum
or Docker in Action in the Cloud/Virtualization forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Defensive copies, Immutable clasess, HashMaps ArrayList

 
marten kay
Ranch Hand
Posts: 178
Java jQuery Postgres Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi

I am working on implementing Joshua Bloch's Item 15 Minimize mutability, for classes that contain HashMaps and ArrayLists, for which I understand defensive copies need to be made to ensure that references to them to do not leak out through accessors.

So I suppose I am looking for the most efficient idiom to do this.

are these it?

 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24208
35
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If somebody passes the list and map to you as constructor arguments, then you're not safe unless you make a copy on the way in, too, since the caller may retain a reference to those collections.

As far as copying return values: that's one way. Another way is to use the Collections.unmodifiableMap() and unmodifiableList() methods, which return a read-only view of a collection you're holding (presumably cheaper to create then copying the whole collection.) Yet another way would be not to return whole collections in the first place; i.e., provide a "String getFromMap(String)" method, if necessary.
 
marten kay
Ranch Hand
Posts: 178
Java jQuery Postgres Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
great, got it, I think.

I make defensive copies in the constructor.

And if I want to pass a copy that the client can modify then I suppose my example above is the way to go.

Cheers
 
marten kay
Ranch Hand
Posts: 178
Java jQuery Postgres Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Further then,

To ensure I that don't create unnecessary copies of my immutable objects, I should create unmodifiable Maps and Lists in my constructor, and then pass references to these in the getters/accessors.

I think I just answered my own question....
 
marten kay
Ranch Hand
Posts: 178
Java jQuery Postgres Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So really, I should do this


[ November 29, 2008: Message edited by: marten kay ]
 
Tom Johnson
Ranch Hand
Posts: 142
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The only thing about the final solution is that the caller has to know not to try to call put on the map returned. This may not appear until 6 months time when someone else is modifying the calling code and gets an UnsupportedOperationException. If you only exposed an interface containing, say, getFromMap as Ernest said then its clear you cant edit it. Personal taste I suppose...
[ November 30, 2008: Message edited by: Tom Johnson ]
 
marten kay
Ranch Hand
Posts: 178
Java jQuery Postgres Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Tom

In my scenario handing over a complete 'master copy' of the map/array to the client is important. So a method to extract individual values is not that useful.

But you raise a good point, as the client does need to be able to edit the map/array, so I will edit the class to return a fresh modifiable copy of the hashmap/array, this will avoid problems as you suggest.
 
Lyle Ziegelmiller
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

It should be noted that neither of the techniques advocated so far provide truly defensive copies.

Here's what unmodifiableMap does:

public V get(Object key) {
return m.get(key);
}

public V put(K key, V value) {
throw new UnsupportedOperationException();
}

It's really just a decorator class that prevents the altering of the internal Map reference(s), but it still allows access to the original objects, which means that someone (either the holder of the "unmodifiable map", or the holder of the original map) could get an object, and alter its contents.

Even creating a new Map or ArrayList and invoking the addAll method on the original objects doesn't solve this, because it only performs a "shallow copy". See http://javatechniques.com/blog/faster-deep-copies-of-java-objects
 
Steve Luke
Bartender
Posts: 4181
21
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What Lyle says is true in the general case, but in the specific case presented here where the collections are of an immutable type, then there is no need for deep copies. It goes to accent Joshua Bloch's stress on immutable objects when possible - if you use immutable objects inside the collection then you don't have to spend the time or worry about making copies.
 
Lyle Ziegelmiller
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Steve Luke wrote:What Lyle says is true in the general case, but in the specific case presented here where the collections are of an immutable type, then there is no need for deep copies. It goes to accent Joshua Bloch's stress on immutable objects when possible - if you use immutable objects inside the collection then you don't have to spend the time or worry about making copies.


Good point.
 
Stephan van Hulst
Bartender
Pie
Posts: 5406
52
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Don't forget to make the class final, or it won't be truly immutable.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic