CODEWITHSUNDEEP
javaimmutability
Atom
Atom

Reputation: 788

Example of an immutable class with hashmap

I have defined the following classes trying to make "IamImmutable.class" immutable. But when I change hashmap values in TestingImmutability.class after initialising IamImmutable, the changes are applicable to the Hashmap. Hashmap would refer to the same object even if we instantiate it with new HashMap(old). I need to make Hashmap in the instance immutable. I have tried iterating and copying the values as well but that doesn't work. Can anyone suggest on how to proceed?

package string;
import java.util.HashMap;
import java.util.Map.Entry;

public final class IamImmutable {
  private int i;
  private String s;
  private HashMap<String, String> h;

  public IamImmutable(int i, String s, HashMap<String, String> h) {
    this.i = i;
    this.s = s;

    this.h = new HashMap<String, String>();
    for (Entry<String, String> entry: h.entrySet()) {
      this.h.put((entry.getKey()), entry.getValue());
    }
  }

  public int getI() {
    return i;
  }

  public String getS() {
    return s;
  }

  public HashMap<String, String> getH() {
    return h;
  }
}

And a test:

package string;

import java.util.HashMap;
import java.util.Map.Entry;

public class TestingImmutability {

  public static void main(String[] args) {
    int i = 6;
    String s = "!am@John";
    HashMap<String, String> h = new HashMap<String, String>();

    h.put("Info1", "!am@John");
    h.put("Inf02", "!amCrazy6");


    IamImmutable imm = new IamImmutable(i, s, h);

    h.put("Inf02", "!amCraxy7");

    System.out.println(imm.getS() + imm.getI());
    for (Entry<String, String> entry: h.entrySet())
      System.out.println(entry.getKey() + " --- " + entry.getValue());
  }
}

Expected output:

  !am@ John6
Inf02---!amCrazy6
Info1---!am@ John

Actual output:

  !am@ John6
Inf02---!amCraxy7
Info1---!am@ John

Upvotes: 4

Views: 10153

Answers (6)

PRIBAN91
PRIBAN91

Reputation: 97

First of all your variable declarations are wrong. The should be essentially be declared as final. Why final? So that nobody can write setter methods of those variables. Also, it doesn't matter whether you do a shallow or deep copy of your HashMap. The basic rule to send a clone() of the mutable object reference from your immutable class.

I have modified your class with minor changes. Here's the code:

package string;
import java.util.HashMap;
import java.util.Map.Entry;

public final class IamImmutable {
    private final int i;
    private final String s;
    private HashMap<String, String> h;

    public IamImmutable(int i, String s, HashMap<String, String> h) {
        this.i = i;
        this.s = s;

        // It doesn't matter whether you make deep or shallow copy
        this.h = new HashMap<String, String>();
        for (Entry<String, String> entry : h.entrySet()) {
            this.h.put((entry.getKey()), entry.getValue());
        }
    }

    public int getI() {
        return i;
    }

    public String getS() {
        return s;
    }

    @SuppressWarnings("unchecked")
    public HashMap<String, String> getH() {
        // Here's the main change
        return (HashMap<String, String>) h.clone();
    }
}

And your test class was also a bit wrong. I modified it with local and ancestral changes. Here's the test class:

package string;
import java.util.HashMap;

public class TestingImmutability {

    public static void main(String[] args) {
        int i = 6;
        String s = "!am@John";
        HashMap<String, String> h = new HashMap<String, String>();

        h.put("Info1", "!am@John");
        h.put("Inf02", "!amCrazy6");

        IamImmutable imm = new IamImmutable(i, s, h);
        System.out.println("Original values : " + imm.getI() + " :: " + imm.getS() + " :: " + imm.getH());

        h.put("Inf02", "!amCraxy7");
        System.out.println("After local changes : " + imm.getI() + " :: " + imm.getS() + " :: " + imm.getH());

        HashMap<String, String> hmTest = imm.getH();
        hmTest.put("Inf02", "!amCraxy7");
        System.out.println("After ancestral changes : " + imm.getI() + " :: " + imm.getS() + " :: " + imm.getH());

    }
}

Upvotes: 3

Amit Mishra
Amit Mishra

Reputation: 50

HashMap implements Cloneable interface in java so you can simple clone the map and return.

Upvotes: 0

flup
flup

Reputation: 27104

Your test is wrong, you're checking the contents of h, the map that you passed to the constructor and later modified, instead of imm.getH(). If you check the proper map

for (Entry<String, String> entry : imm.getH().entrySet())
        System.out.println(entry.getKey() + " --- " + entry.getValue());

things look just fine:

!am@John6
Info1 --- !am@John
Inf02 --- !amCrazy6

So your IamImmutable constructor is already fine, any later changes to the original map passed into the constructor won't affect the copy you made at construct time. You can also use the other HashMap constructor you mentioned, which is slightly more readable:

public IamImmutable(int i, String s, HashMap<String, String> h)
{
    this.i = i;
    this.s = s;
    this.h = new HashMap<String, String>(h);
}

And that will work fine too.

A different problem is that getH() passes a reference to the internal map out to the world, and if the world changes that reference, things will go wrong. An easy way to get around that is to apply the same copy trick that you already use in the constructor in getH() as well: 

public HashMap < String, String > getH() {
    return new HashMap<String,String>(h);
}

Or to decorate the internal map before returning it:

public Map<String, String> getH() {
    return Collections.unmodifiableMap(h);
}

Consider using the ImmutableCollections in the guava library instead. They've done the same job that this code is doing, but with a bit more thought to efficiency and ease of use. The full copies at construct time and to get the map are unwieldy and the standard underlying HashMap will do modification checks that are pointless if we know it isn't going to get modified anyways.

Upvotes: 3

slipperyseal
slipperyseal

Reputation: 2778

check out

<K,V> Map<K,V> java.util.Collections.unmodifiableMap(Map<? extends K,? extends V> m)

but note this creates a read only view onto the original map. so you might want to copy the input map.

http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html?is-external=true#unmodifiableMap%28java.util.Map%29

this should work…

public IamImmutable(int i,String s, Map<String,String> h) {
    this.i = i;
    this.s = s;

    Map<String,String> map = new HashMap<String,String>();
    map.putAll(h);
    this.h = Collections.unmodifiableMap(map);
}

you may also probably need to change the member declaration and get methods to the base Map type, rather than HashMap

Upvotes: 3

Ashok kumar
Ashok kumar

Reputation: 195

HashMap <MyKey, MyValue> unmodifiableMap = Collections.unmodifiableMap(modifiableMap);

Use above code for immutable object.

Upvotes: 0

Eran
Eran

Reputation: 393771

In order for your class to be immutable, getH() must return a copy of the HashMap. Otherwise, any caller of getH() can modify the state of the HashMap member of your IamImmutable class, which means it's not immutable.

An alternative would be to replace getH() with methods that access that internal HashMap without exposing it. For example, you can have a method String[] keys() that returns all the keys of the HashMap and a method String get(String key) that returns the value for a given key.

Upvotes: 3

Related Questions