Bug in using Object.clone()

Question

I have the next scenario:

I define an int[][] variable in my main class. int[][] matrix1 = new int[10][10] and i give it some values. I then call a method and i send this variable as a parameter to that method. Being an object it sends is by reference not by value, so inside the method, because i have to change the values contained by matrix1 but not affect the object after it returns from the method, i make a clone of it like so:

private void myMethod( int[][] matrix1 )
{
    int[][] matrix1Clone = matrix1.clone();
    //And next i do some changes to matrix1Clone
    ......
}

But the problem is that the changes i do to matrix1Clone also happen in matrix1. So it hasn't really created a clone of matrix1 object, but both variables point to the same object.

Why is this? I can't seem to figure it out. Why doesn't clone method work?

If you need more info, please ask. But i'm afraid this is about it, can't really give you more, but maybe i could try.

I might be missing something, but i can't figure out what...

Thanks.

EDIT

Sorry, made a typo. It's late hre and i'm tired. I'm using clone method indeed, that's why i'm confused as it's not working :(.

Answer 1

Actually, arrays have no values but pointers towards object or primitive datatypes. If you want a detailed answer, you should read my commentary here: Java is NEVER pass-by-reference, right?...right? or here: In Java, what is a shallow copy?

So, as arrays are pointers, what happens if you clone a pointer with pointers in it? At first, the pointers are copied for real, but these pointers only point toward other object which aren't cloned. So if you want to clone, I suggest not using arrays but "harder" data structures: classes. Another possibility would to never store an array within an array...like I use arrays only for containers!

But I can't give you details about Java multidimensional generics, as I never deal with them, not only because of their possible inconsistency because they are arrays (they're violating some OO principles anyway and make code looking ugly).

EDIT

I was running a few tests how the clone method works for arrays inside a class, what the problem is and which workarounds we have.

First the test data structure:

public class Foobar implements Cloneable {
    String[] array;

    public Foobar() {
        this.array = new String[10];
    }

    public String getValue(){
        return array[0];
    }

    public String[] getArray(){
        return array;
    }

    public void setArray(String[] array){
        this.array = array;
    }

    @Override
    public Object clone(){
        try{
            Foobar foobar = (Foobar) super.clone();
            foobar.setArray(array);
            return foobar;
        }
        catch(Exception e){
            return null;
        }
    }
}

Now the controller:

String[] array = new String[10];
array[0] = "111";
Foobar foo1 = new Foobar();  
foo1.setArray(array);
Foobar foo2 = foo1; //Alternation: Foobar foo2 = (Foobar) foo1.clone();  
System.out.println("Instance: "+foo1.getArray()+" with value: "+foo1.getValue());
System.out.println("Instance: "+foo2.getArray()+" with value: "+foo2.getValue());
array[0] = "999";
System.out.println("Instance: "+foo1.getArray()+" with value: "+foo1.getValue());
System.out.println("Instance: "+foo2.getArray()+" with value: "+foo2.getValue());

The test results will always look like that - no matter if I use = or clone():

Instance: [Ljava.lang.String;@42e816 with value: 111
Instance: [Ljava.lang.String;@42e816 with value: 111
Instance: [Ljava.lang.String;@42e816 with value: 999
Instance: [Ljava.lang.String;@42e816 with value: 999 

This is not good!!

So what is the workaround? I suggest doing this in every data structure class:

public class Foobar implements Serializable {
    //any class variables...it doesn't matter which!

    public Foobar() {
        //do initialisation here...it doesn't matter what you do!
    }

    public Foobar copy(){
        try{
            ByteArrayOutputStream baos = new ByteArrayOutputStream();
            ObjectOutputStream oos = new ObjectOutputStream(baos);
            oos.writeObject(this);
            ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
            ObjectInputStream ois = new ObjectInputStream(bais);
            Foobar foobar = (Foobar) ois.readObject();
            return foobar;
        }
        catch(Exception e){
            return null;
        }
    }
}

So you will get a full copy by implementing just one line of code:

Foobar foo2 = foo1.copy(); //nice and easy!!

The advantage of this solution: It's usually enough to implement the interface Serializable to make a class "copyable". And if not, you can solve any issues by reading what is written in the Serializable Javadoc!

Even more: It doesn't matter what kind of objects are in the class you want to make "copyable", so you don't need to spend any more time on this issue. After all, above code is the simpliest and fastest solution deeply embedded in Java ever since and uses only RAM! (thanks to ByteArrayOutputStream)

Enjoy!

UPDATE: Note that you only need to use an object's copy if you want a temporary stack or if you are dealing with threads (in general: if you need to have objects fully independent from each other). Otherwise you shouldn't make any copy at all! Also if you write some data into a file or a socket, you don't need a copy. Even more I suggest to implement the copy method only when it's really used: for data structures (model). So be careful by using this mighty method (otherwise it could slow down your app, or even fill up the Java VM storage if you make millions of copies with no reason, this would cause a stackoverflow indeed :o).

EDIT

I was working a bit more on the this issue. Because I suddenly found out, that there is a public clone() method of "primitive" arrays that aren't in the Java API !! (a "easter egg" from SUN for arrays like String[] or int[] ;-)

And as I use real arrays as the basic data structure of Foobar (not ArrayLists!), I can change the clone method (of above class) like this:

@Override
public Object clone(){
    try{
        Foobar foobar = (Foobar) super.clone();
        String[] arrayClone = array.clone(); //who thought that this is possible?!
        foobar.setArray(arrayClone);
        return foobar;
    }
    catch(Exception e){
        return null;
    }
}

And now we get this result right out of the box:

Instance: [Ljava.lang.String;@42e816 with value: 111
Instance: [Ljava.lang.String;@9304b1 with value: 111
Instance: [Ljava.lang.String;@42e816 with value: 999
Instance: [Ljava.lang.String;@9304b1 with value: 111

Problem solved with "double-nested" objects!!! As you can see, the clones have different objects independently from the original...therefore foo1.equals(foo2)) will be false!

Solution: In the clone method of a class, you need to clone all its class variables, too! (But if some class variables are ArrayLists or more-dimensional arrays, even this solution won't work!)

Finally, what is the real issue? The class ArrayList doesn't clone it's arrays, it only calls the method copyOf in the class Array, which is harmful. So never use the clone method of the class ArrayList, and never inherit any class from ArrayList because its clone method won't work! (It works only if the class ArrayList only contains primitives and no objects...otherwise just use the easy ByteArray solution above!).

Note that with more-dimension arrays like Object[][] you always need to implement the ByteArray solution above, they can't be cloned! And if your array is huge, it may take a while and need some RAM, too.

Now you are a cloning expert! :-D

Answer 2

You are giving matrix1Clone same reference as matrix1. If you change matrix1Clone then matrix1 changes too.

You can copy your array with iterating over the source array:

   public static int[][] clone2DArray(int[][] array) {
        int rows = array.length;

        //clone the 'shallow' structure of array
        int[][] newArray = array.clone();
        //clone the 'deep' structure of array
        for(int row = 0; row < rows; row++){
            newArray[row] = array[row].clone();
        }

        return newArray;
    }

Answer 3

Try clone it using clone() http://docs.oracle.com/javase/1.4.2/docs/api/java/lang/Object.html#clone%28%29

private void myMethod( int[][] matrix1 )
{
    int[][] matrix1Clone = matrix1.clone();
}

or, copy all of the values using a loop

EDIT: Api for clone() says it should return a copy of the object, but behavior might be different depending on which object's beeing cloned. Try iterating over the array as an alternative. Since it's a 2d array, you need a nested loop:

for(int i=0; i<old.length; i++)
  for(int j=0; j<old[i].length; j++)
    old[i][j]=copy[i][j];

where old is the "original array" and copy is the copy