Clone Object into array list, java

Question

I have an object I created that has a number of properties, including a jagged array property, called matrix.

In my script, I want to clone a copy of the object and place it into an arraylist of my object. however i am not able to get a cloned copy of the matrix property to set correctly, as it keeps passing the last known reference into my list. Code below

MatrixObject newMatrixObject = new MatrixObject();
List<MatrixObject> listMatrix = new ArrayList<MatrixObject>();

try {  //some code here looking int text file

    int[][] myArray = some value;
    newMatrixObject.matrix = myArray; //this works but keeps changing to the last value of myArray
    //I tried this as well
    newMatrixObject.matrix = newMatrixObject.SetMatrix(myArray); // didnt work either and I tried setting it without returning an array, same story
    listMatrix.add(new MatrixObject(newMatrixObject));
}

...and for the object class I have been doing a bunch of things, but generally this

public class MatrixObject 
{
    public Date startDate;
    public int[][] matrix;

    public MatrixObject (MatrixObject copy) {

        this.startDate = copy.startDate;
        this.matrix = copy.Matrix;
}

I also created this method in the class, but I dont think its working

public  int[][] SetMatrix(int[][] inputMatrix){
    //if (inputMatrix == null){
    //return null;
    //}

    int [][] result = new int [inputMatrix.length][];
    for ( int i= 0; i< inputMatrix.length; i++)
    {
       result[i] = Arrays.copyOf(inputMatrix[i], inputMatrix[i].length);
    }

    System.out.println(Arrays.deepToString(result));
    return result;
}

If there is a better way to add a clone of the object into the list, that will work as well. I am easy, just trying to figure this thing out.

Answer 1

Using new followed by the class name to copy an object often leads to code that is not extensible. Using clone, the application of the prototype pattern, is a better way to achieve this. However, using clone as it is provided in Java can be problematic as well.

It is better to invoke a non-public copy constructor from the clone method. This gives us the ability to delegate the task of creating an object to an instance of a class itself, thus providing extensibility and also, safely creating objects using the non-public copy constructor.

Below is a reworking of your MatrixObject class. It shows how you can implement clone safely by relying on the construction process. This is especially useful in cases where your class contains final fields.

MatrixObject Class

import java.util.*;

public class MatrixObject implements Cloneable {
    private Date startDate;
    private int[][] matrix;

    public MatrixObject(Date newDate, int[][] newMatrix) {
        this.startDate = newDate;
        this.matrix = newMatrix;
    }

    protected MatrixObject(MatrixObject another) {
        Date refDate = null;
        int[][] refMatrix = null;

        refDate = (Date) another.startDate.clone();
        refMatrix = another.matrix.clone();

        this.matrix = refMatrix;
        this.startDate = refDate;
    }

    public void setMatrix(int[][] newMatrix) {
        this.matrix = newMatrix;
    }

    public void setDate(Date newDate) {
        this.startDate = newDate;
    }

    public String toString() {
        String s = "";

        for (int[] tmp : this.matrix) {
            s += Arrays.toString(tmp) + "\n";
        }

        return String.format("%s%n%s", this.startDate.toString(), s);
    }

    @Override
    public Object clone() {
        return new MatrixObject(this);
    }

    // MAIN GOES HERE (or anywhere in this class that is outside of a method)
    // static void main(String[] args) { ... }

}

Main Method

public static void main(String[] args) {
    String output = "";
    Calendar dates = Calendar.getInstance();
    Date dateOne = dates.getTime();
    // offset day of the month by one day
    dates.set(Calendar.DAY_OF_MONTH, dates.get(Calendar.DAY_OF_MONTH) + 1);
    Date dateTwo = dates.getTime();

    int[][] myArrayOne = { { 1, 0, 0 }, { 0, 1, 0 }, { 0, 0, 1 }, };
    int[][] myArrayTwo = { { 0, 0, 1 }, { 0, 1, 0 }, { 1, 0, 0 }, };

    // create two MatrixObjects, an original and a copy
    MatrixObject oneMO = new MatrixObject(dateOne, myArrayOne);
    MatrixObject copyMO = (MatrixObject) oneMO.clone();

    // show the contents of the original MatrixObject and its copy
    output += String.format("First MatrixObject:%n%s%n", oneMO.toString());
    output += String
    .format("Copied MatrixObject:%n%s%n", copyMO.toString());

    // alter the original MatrixObject
    oneMO.setMatrix(myArrayTwo);
    oneMO.setDate(dateTwo);

    // show that alterations to the original MatrixObject did not
    // effect the copy
    output += String.format("Changed First MatrixObject:%n%s%n",
    oneMO.toString());
    output += String.format("Unchanged Copied MatrixObject:%n%s%n",
    copyMO.toString());

    System.out.println(output);
}

Output

First MatrixObject:
Mon Apr 20 21:29:14 EDT 2015
[1, 0, 0]
[0, 1, 0]
[0, 0, 1]

Copied MatrixObject:
Mon Apr 20 21:29:14 EDT 2015
[1, 0, 0]
[0, 1, 0]
[0, 0, 1]

Changed First MatrixObject:
Tue Apr 21 21:29:14 EDT 2015
[0, 0, 1]
[0, 1, 0]
[1, 0, 0]

Unchanged Copied MatrixObject:
Mon Apr 20 21:29:14 EDT 2015
[1, 0, 0]
[0, 1, 0]
[0, 0, 1]

Answer 2

It's hard to tell exactly what you are doing, but I think the problem is the constructor.

public MatrixObject (MatrixObject copy) {

    this.startDate = copy.startDate;
    this.matrix = copy.matrix;
}

This makes a MatrixObject that shares the internals of copy, so any changes you make to either to the new or the old MatrixObject will actually change the other one. Instead you should copy all of the fields, like this:

public MatrixObject (MatrixObject copy) {

    this.startDate = new Date(copy.startDate.getTime());
    this.matrix = new int[copy.matrix.length][];
    for (int i = 0; i < copy.matrix.length; i++)
        this.matrix[i] = Arrays.copyOf(copy.matrix[i], copy.matrix[i].length);
}