0

I have this:

import java.util.ArrayList;


public class MyClass {
    ArrayList list = new ArrayList();

    public MyClass(){

    }

    public MyClass(MyClass aClass){
        this.list = aClass.getList();
    }

    public void add(int number){
        list.add(number);
    }

    public void set(int number){
        list.set(0, number);
    }

    public int get(){
        return (Integer)list.get(0);
    }

    public ArrayList getList(){
        return list;
    }
}



MyClass aName = new MyClass();
aName.add(5);
System.out.println("aName: "+aName.get());

MyClass aName2 = new MyClass(aName);
System.out.println("aName2: "+aName2.get());

aName2.set(1);
System.out.println("aName2: "+aName2.get());
System.out.println("aName: "+aName.get());

This will print: aName: 5 aName2: 5 aName2: 1 aName: 1

I don't want my second object changing the values in my first object. Is there any way to stop this happening but still be able to copy properties from another object?

Undefined
  • 1,899
  • 6
  • 29
  • 38
  • 2
    You need to make a copy of the internal ArrayList, not just assign the reference. e.g. `this.list = aClass.getList().clone()` – nhahtdh Jul 26 '12 at 19:37
  • Why exactly are you homerolling your own arraylist? – Wug Jul 26 '12 at 19:41
  • @wug he isn't, it's creating behavior around an array list. – corsiKa Jul 26 '12 at 19:43
  • Hmm, you're right. He's homerolling a stack with an arraylist. Does inserting to the beginning of an arraylist operate in O(N) time? – Wug Jul 26 '12 at 19:44

4 Answers4

1

Then don't pass the List from the original object to the new object, in

public MyClass(MyClass aClass){ 
    this.list = aClass.getList(); 
} 

It makes both object have a reference to the same list (there is only one list shared between both objects).

You should do

public MyClass(MyClass aClass){ 
    this.list = (List) aClass.getList().clone(); 
} 
SJuan76
  • 24,532
  • 6
  • 47
  • 87
1

Yes you can stop this. You need to make defensive copy of the list in your copy constructor.

public MyClass(MyClass aClass){
    this.list = new ArrayList(aClass.getList());
}

You could use clone(), but as Josh Bloch states in Effective Java item "copy constructors have many advantages over clone" - they're preferable in practice.

corsiKa
  • 81,495
  • 25
  • 153
  • 204
1

You could clone() or take a copy, but since you have created a list already, rather than discarding it you could use it. ;)

public MyClass(MyClass aClass){
    this.list.addAll(aClass.getList());
}
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 2
    That's true, he's creating one on declaration. Another reason why `final` variables are useful - that would have detected this bug on compile! – corsiKa Jul 26 '12 at 19:44
0

You're copying the reference value of the ArrayList into your new object. You now have two references to the same list.

You need to create a new ArrayList when you construct your second object:

public MyClass(MyClass aClass){
    this.list = new ArrayList(aClass.getList());
}

Be aware, however, that both lists now contain the same objects. Again, this only creates a new list and copies the reference values of the objects inside the original list. If you needed two complete and separate lists containing unique objects you would need to write a method that did a deep copy, duplicating the objects in the list as well.

Brian Roach
  • 76,169
  • 12
  • 136
  • 161