Copying an Object in Java without affecting the original via copy constructor

Question

I'm trying to copy an Object, which will then be modified, without changing the original object.

I found this solution and it seemed the best approach would be a copy constructor - from my understanding, this would give me a deep copy (a completely separate object from the original).

So I tried that. However, I've noticed that when the following code executes, it's effecting all previous objects from which it was copied. When I call surveyCopy.take(), which will change values inside of the Survey, it's also changing the values inside of selectedSurvey.

public class MainDriver {
...
//Code that is supposed to create the copy
case "11":  selectedSurvey = retrieveBlankSurvey(currentSurveys);
            Survey surveyCopy = new Survey(selectedSurvey);
            surveyCopy.take(consoleIO);
            currentSurveys.add(surveyCopy);
            break;
}

and here is code for my copy constructor:

public class Survey implements Serializable
{
    ArrayList<Question> questionList;
    int numQuestions;
    String taker;
    String surveyName;
    boolean isTaken;

    //Copy constructor
    public Survey(Survey incoming)
    {
        this.taker = incoming.getTaker();
        this.numQuestions = incoming.getNumQuestions();
        this.questionList = incoming.getQuestionList();
        this.surveyName = incoming.getSurveyName();
        this.isTaken = incoming.isTaken();
    }
}

So what exactly is the issue? Does a copy constructor not work that way? Is the way I coded it wrong?

Answer 1

This is the problem, in your copy constructor:

this.questionList = incoming.getQuestionList();

That's just copying the reference to the list. Both objects will still refer to the same object.

You can use:

this.questionList = new ArrayList<Question>(incoming.getQuestionList());

to create a copy of the original list - but this is still not good enough if Question itself is mutable. In that case, you'd have to create a copy of each Question object to achieve total isolation.

Your other fields are okay, as they're either primitives or references to String (which is immutable, allowing you to share references safely).

Answer 2

This

this.questionList = incoming.getQuestionList();

most likely copies the reference to the original list (I say probably since it's possible that getQuestionList() gives you a defensive copy). You will probably have to make a new copy of that list. And perhaps the contained Question objects. And perhaps anything that they reference.

This is the problem with deep copies. In order to do this reliably you have to copy all mutable objects. Note that if an object is immutable (e.g. the Strings) then they can't be changed and so you can reference the originals confident that they won't be changed. The same applies to primitives. One good reason for encouraging immutability in your code base.

If you can't make an immutable class, write your class such that it makes defensive copies. i.e. when a client asks it for a collection, it should make a copy and return that. Otherwise your supposedly well-meaning clients could alter your internal state (inadvertently or otherwise).