3

I was implementing an immutable class whose structure is as below :

public final class A{
    private final B bOb;
    public A(){
        bOb = new B();
    }
    public A(A a){
        bOb = new B(a.bOb);
    }
    public A addData(Type data){ // Type - Integer,String,char,etc.
        A newA = new A(this); //making a copy of the object that is calling addData method
        newA.bOb.add(data);
        return newA;
    }
}

Is this implementation correct ? Let's say the object bOb is a list.

h4ck3d
  • 6,134
  • 15
  • 51
  • 74

5 Answers5

2

Your implementation:

public A addData(int data){
    A newA = new A(this);
    newA.bOb.add(data); // <--- bOb is mutable
    return newA;
}

A bOb is being changed, so if you expose any direct access to bOb, it can be changed. (Since you're not exposing anything about bOb, the scenario doesn't make sense.)

Hints:

  • Collections.emptyList() returns an immutable list.
  • Collections.unmodifiableList() returns an immutable shallow-copy of a given list.

Consider this "safer" implementation:

import java.util.*;

public final class A<T>{ //T must also be immutable (String, integer, char, ...)
    private final List<T> list;
    public A(){
        this.list = Collections.emptyList();
    }
    public A(List<T> list){
        this.list = Collections.unmodifiableList(list);
    }
    public A<T> addData(T data){
        List<T> shallowCopy = new ArrayList<T>(this.list);
        shallowCopy.add(data);
        return new A<T>(shallowCopy);
    }
    public List<T> getItems() {
        return this.list;
    }
}
EthanB
  • 4,239
  • 1
  • 28
  • 46
  • It depends on how `new B(a.bOb);` is implemented. Or not? – gkuzmin Aug 23 '12 at 09:51
  • Try [Collections.unmodifiableList()](http://docs.oracle.com/javase/1.4.2/docs/api/java/util/Collections.html#unmodifiableList(java.util.List))... does it still work? – EthanB Aug 23 '12 at 09:53
  • Mutating `bOb` within the class is usually considered fine *as long as the state change is invisible outside of `A`*. See [this description](http://www.artima.com/pins1ed/type-parameterization.html#lst:simple-functional-queues) of immutable queues in Scala for a similar example. – DaoWen Aug 23 '12 at 09:53
2

It is not totally immutable, since B appears to be mutable itself (via the add method) and A contains an instance of B.

However I think it is effectively immutable (i.e. it behaves as if it was immutable from the perspective of an external observer) providing that all the following are true:

  • new B(B) performs a complete deep copy of B (if not then addData may mutate something within the original B)
  • you aren't leaking references to bOb via any other means
  • the elements added themselves are immutable

You get most of the benefits of immutability from being effectively immutable so I think this design is OK providing Type is immutable - it's fine to mutate an object during it's construction providing it never gets mutated after you pass a reference to someone else. A good example of this is java.lang.String - internally it contains a mutable array that is written to while the String is constructed but never changed after that point.

mikera
  • 105,238
  • 25
  • 256
  • 415
  • Since B in my case is a LinkedList (which is mutable) , and I am doing , `bOb = new B(a.bOb);` , that is , I am not using any getter for bOb , I am accessing it directly through my object `A a` , it should be immutable right? – h4ck3d Aug 23 '12 at 10:02
  • @sTEAK - LinkedList doesn't do a deep clone, so if any of its contents are mutable then you lose your overall immutability. If you stick to immutable contents in the LinkedList then you should be fine - but that rather depends on `Type`. P.S. might be a good place to use generics :-) – mikera Aug 23 '12 at 10:06
  • Is there any work-around for adding mutable objects to this ? So that it still stays immutable – h4ck3d Aug 23 '12 at 10:07
  • @sTEAK - the only workaround is to ensure that no-one else keeps a reference to the contained mutable objects. In practice, this is tricky to guarantee unless you control all the other code that uses class A so be careful. – mikera Aug 23 '12 at 10:12
  • Let's say I add , Date object to this list , and then I change the date through setDate method , will that change be reflected in my list too? I don't think so – h4ck3d Aug 23 '12 at 10:15
  • Yes, calling of `setDate` will affect `Date` object and `bob` too, because `List` keep only reference to this object, not copy. But sinse your class does not share `bob` or its content then nobody outside `A` can see this. – gkuzmin Aug 23 '12 at 10:17
  • @gkuzmin You have overlooked the fact that the Date object came from outside so it is shared far and wide. – Marko Topolnik Aug 23 '12 at 10:21
  • 2
    @sTEAK. You can't possibly have an immutable object that takes foreign objects and makes its observable state depend in any way on the state of those objects. So the question is still open: does the observable state of your A depend in any way on the state of the objects in the LinkedList? – Marko Topolnik Aug 23 '12 at 10:25
  • Lets say, that you have `@Override public String toString()` method in your class that print `bob` content. Then after `date` object modification `toString()` method will return different string representation and this means that changing of `date` and so `bob` objects state can be visible from outside of `A` and here we have the conclusion - it is mutable. – gkuzmin Aug 23 '12 at 10:25
  • 1
    Defencive copy technique can help you. For example, you can add some interface `Copyable>` wich will have `public T getCopy()` method. And you class can take this interface as generic parameter and use it for defencive copy. But this solution have disadvatages - only your classes can implement this interface. – gkuzmin Aug 23 '12 at 10:42
  • @MarkoTopolnik: That is very true, though many people fail to consider the importance of the phrase "observable state". It is perfectly valid for an immutable object to hold a reference to a publicly-mutable one if the state of the object holding the reference is considered to include the *identity* of the referred-to object but not its *current state*. It's too bad neither Java nor most .net languages makes any distinction between fields which logically "own" the referred-to object, versus those which *identify* objects that are owned by something else. – supercat Aug 23 '12 at 17:21
1

If bOb is a list and it contains mutable content, then not. But it seems that you use only int as content and this solves the problem.

gkuzmin
  • 2,414
  • 17
  • 24
  • Well B is actually a LinkedList and I am adding a generic value to the list through addData method , so it is not necessarily an `int`. So you mean to say its not immutable? – h4ck3d Aug 23 '12 at 09:51
  • For example, if you put mutable content in your `bOb` (lets say another `List`), then it can be modified and this will affect A instance state. – gkuzmin Aug 23 '12 at 09:53
  • But if I am adding an `Integer` to the list throw the `addData` method , then ? – h4ck3d Aug 23 '12 at 09:55
  • `Integer` is immutable, so your class will be immutable for this type. But if you will use `Date`,for example, then it can be modified and affect instance state. For example, `A a=new A();Date d=new Date();a.addData(d);d.setTime(1L);`. In last operator `d` object will be modified and `a` object state will be modified too. But this modification will not affect visible state since your class does not have it. This is interesting part -you class does not have visible state, so it is useless, but immutable. If you will share its state (including `bob` value), then visible state will be affected. – gkuzmin Aug 23 '12 at 10:02
  • So , effectively is it immutable in all cases? – h4ck3d Aug 23 '12 at 10:04
  • I think that answer is yes, until you will share `bob` or its content somehow. If someone can get `bob` content then he can see state changings in case of mutable data. – gkuzmin Aug 23 '12 at 10:15
  • That is my point - until you does not share the state your class is immutable and useless. But when you will share `bob` ot its content your class will become mutable but useful. – gkuzmin Aug 23 '12 at 10:20
1

In this form, yes, your class is immutable.

However, this is a trivial example and wouldn't actually be useful for anything since you can't access any of the internal data. The thing you need to be careful of is to not let any references to bOb escape from A. If you add a method which returns bOb, you need to return a copy of bOb to avoid accidentally allowing the caller to mutate any of the contents of A.

DaoWen
  • 32,589
  • 6
  • 74
  • 101
0

It depends on how B(B b) constructor works. If it is a copy constructor, it should make a deep copy of b fields. In this case A is immutable.

Instead if the constructor simply take the reference to the same b instance, any change to it will reflect on bOb property, so the A class in not immutable...

davioooh
  • 23,742
  • 39
  • 159
  • 250
  • well, so using `LinkedList(Collection c)` you can obtain a new list with the same elements. As you say `Type` is a primitive type (or a wrapper), so your `A` class is immutable. – davioooh Aug 23 '12 at 10:53