Copying Array, Allow it to remain the same when input to constructor changes

Question

My point class is immutable. When I am given input into the constructor originally, it should be copied into the cloneList object. This would allow for it to remain the way it was before if a couple of indexes in the array are changed via the constructor. I have tried almost every possible combination and am still running into the trouble. I want the cloneList to be a copy of the original Point[] points array, thus if the points array is changed then the cloneList will not.

import java.util.Arrays;
import java.util.Iterator;
 import java.util.List;

public class  Polygon {

private double xSum = 0;
private double ySum = 0;
private Point[] points;
private Point[] cloneList;
private Point a;

public Polygon(Point[] points) {

    this.points = points;
    cloneList = new Point[points.length];
    for (int i = 0; i < cloneList.length; i++) {
        cloneList[i] = points[i];
    }



    for (int i = 0; i < cloneList.length; i++)
        System.out.println(cloneList[i]);


     for (int i = 0; i < points.length; i++){
    cloneList[i] = points[i];

    // System.out.print(cloneList[i].getX());
    // System.out.print(cloneList[i].getY());
    // System.out.println();
}

public Point getVertexAverage() {
    double xSum = 0;
    double ySum = 0;
    for (int index = 0; index < cloneList.length; index++) {
        xSum = xSum + cloneList[index].getX();
        ySum = ySum + cloneList[index].getY();
    }

    return new Point(xSum / getNumSides(), ySum / getNumSides());
}

public int getNumberSides() {
    return cloneList.length;
}

}

Two things: 1) do you really expect that we read all your code to understand your problem? Maybe it's not necessary **all the code**, so just post the relevant part of it. 2) You need to return a new list (that you call clone) as **clones** of your points, this means, a totally new `Point` (or whatever) object with the same data as the original point (note: same data **doesn't mean** same references to objects/other data) and you're not doing that anywhere since you're just using the same `Point` reference in both your `points` and your `cloneList` arrays: `cloneList[i] = points[i]`. — Luiggi Mendoza, Apr 23 '13 at 04:17

score 1 · Answer 1 · answered Apr 23 '13 at 04:19

You have to write a copy method to copy every variable from one object into a new object, and use that new object. There is no way to dereference references in Java (i.e. pass by value) except to copy.

Something like:

public Point copy(){
 Point temp = new Point();
 temp.setX(this.getX());
 ....Add in the rest of the assignments.
 return temp;
}

score 1 · Accepted Answer · edited May 23 '17 at 12:28

Based on my comment on your post, an option to solve your problem will be to add new Point instances to your cloneList array:

public PolygonImpl(Point[] points) {
    //some code here...
    for (int i = 0; i < cloneList.length; i++) {
        //create the new Point instance (the clone) here
        //this code is just an example since you haven't provided the Point constructor
        cloneList[i] = new Point(points[i].getX(), points[i].getY());
    }
    //some more code here...
}

But this option is a little clumsy because when providing the cloneList attribute to a client of your Polygon interface (or the PolygonImpl class) it will modify the array (since arrays are mutable) and the original cloneList will be modified as well. Knowing this, it would be better to do not have a cloneList as attribute, instead create this list on a method:

public Point[] getPoints() {
    Point[] cloneList = new PointList[X]; //where X is some size you know
    for (int i = 0; i < cloneList.length; i++) {
        //create the new Point instance (the clone) here
        //this code is just an example since you haven't provided the Point constructor
        cloneList[i] = new Point(points[i].getX(), points[i].getY());
    }
    return cloneList;
}

I promote this option since your Point class seems to have few data in it. For real world applications where your classes will be more complex (like having an internal List of objects that contains more objects instances making a complex tree) you should not create something like this (because it will be a living hell), instead use a copy/clone method. For this, you can use some of the techniques provided here: Java: recommended solution for deep cloning/copying an instance

For the second part, creating a method for the cloneList... If there is no set value for the array at the time of the test, would using Point[] cloneList = new PointList[points.length] be appropriate? — Fish, Apr 23 '13 at 04:34
@Fish only if you know that `points.length` will be the max size of the array of clones. — Luiggi Mendoza, Apr 23 '13 at 04:35
If I were making a separate method to contain this duplicate array, how would this be dealt with when other methods need to values in cloneList? Also, Initialize the list at the top or just in the method? — Fish, Apr 23 '13 at 04:39
@Fish using the second method would mean that every client who gets a copy of the internal `Point[]` array won't mess with each other. In your first attempt, if two clients get the same `Point` array and one of them modifies the array contents, the array reference in the other client will be modified as well. — Luiggi Mendoza, Apr 23 '13 at 04:42

score 0 · Answer 3 · edited May 23 '17 at 10:25

You are not making any copies of the actual Points in your code. Remember that most values in Java are references, so while it may seem like you are making copies of your Point, you are only copying pointers around.

There are a couple of things you can do. You can use varying methods to clone your objects (see Deep copy, shallow copy, clone, for instance). The other way is to define your Points as being immutable (much like the String class):

public class Point {
    private final float x;
    private final float y;

    public Point(float x, float y) {
        this.x = x;
        this.y = y;
    }

    public float getX() {
        return this.x;
    }

    public float getY() {
        return this.y;
    }
}

This would force all client code to treat points as immutable. You would then define operations on Points as returning new Points.

Copying Array, Allow it to remain the same when input to constructor changes

3 Answers3