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?

score 14 · Answer 1 · answered Dec 06 '12 at 14:35

14

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).

answered Dec 06 '12 at 14:35

Jon Skeet

1,421,763
867
9,128
9,194

Thanks for your response. It sounds like I should make `Question` immutable, because I'm not sure if it is. How can I do that? Edit: Maybe that isn't the best idea, because inside Question are more objects that are immutable. I'm kind of stumped. – iaacp Dec 06 '12 at 14:52
@iaacp: Yes, if you can make `Question` immutable, that would help. If all its fields are already of immutable types, that should make it *easier* rather than harder. – Jon Skeet Dec 06 '12 at 15:01

Brian Agnew · Answer 2 · 2012-12-06T14:40:04.437

8

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).

edited Dec 06 '12 at 14:40

answered Dec 06 '12 at 14:34

Brian Agnew

268,207
37
334
440

It *definitely* copies the reference – mishadoff Dec 06 '12 at 14:35
I take your point. I was trying to indicate that the method could return a defensive copy (i.e. it would create a copy and then return it). Regardless, a reference copy would occur somewhere – Brian Agnew Dec 06 '12 at 14:38
@mishadoff - not necessarily. We haven't been shown the getter code, and some getters do actually create a defensive copy (which would work fine for a deep clone). "most likely copies the reference" is a correct interpretation, I think. – mikera Dec 06 '12 at 14:38
Thank you for your response. Is there any down side to making all objects immutable? How do I go about making them immutable? – iaacp Dec 06 '12 at 17:01

score 5 · Answer 3 · answered Dec 06 '12 at 14:40

The problem when creating deep copies is the fact that everything that is not a primitive type is copied by reference unless you are using a specific deep copy constructor on it too.

In your specific case you have no problems with bool, int or String variables because you pass them around by value (actually String is passed by reference but it's immutable so there is no problem) but you are passing a ArrayList<Question> questionList. When you do

this.object = incoming.object

you just copy a reference. So both variables point to the same object in memory, so you are not deeply copying it. You must create another instance of the object that has the same inner values, then you will be sure, eg this.object = new YourObject(incoming.object).

Mind that usually means that more is your class complicated in the composition tree, more you will have to go deep in the variables until you copied them all.

score 0 · Answer 4 · answered Jun 01 '19 at 12:54

If we need to copy a simple pojo (Not nested). Then shallow copy is enough.

Cloner class

import java.lang.reflect.Field;

public class Cloner {
    public static <T> T cloneShallow(T srcEntity, T destEntity){
        try {
            return copy(srcEntity, destEntity);
        }catch (Exception e){
            e.printStackTrace();
        }
        return null;
    }

    private static <T> T copy(T srcEntity, T destEntity) throws IllegalAccessException, InstantiationException {
        if(srcEntity == null){
            return null;
        }

        Class<?> clazz = srcEntity.getClass();

        T newEntity;

        if(destEntity != null){
            newEntity = destEntity;
        }else{
            //create new instance
            newEntity = (T) srcEntity.getClass().newInstance();
        }

        while (clazz != null) {
            copyFields(srcEntity, newEntity, clazz);
            clazz = clazz.getSuperclass();
        }

        return newEntity;
    }

    private static  <T> T copyFields(T entity, T newEntity, Class<?> clazz) throws IllegalAccessException {
        for (Field field : clazz.getDeclaredFields()) {
            field.setAccessible(true);
            field.set(newEntity, field.get(entity));
        }
        return newEntity;
    }
}

Lets call..

eg.
Apple apple = new Apple();
apple.setColor("Green");

Apple newApple = Cloner.cloneShallow(apple, new Apple());
( or )
Apple newApple = Cloner.cloneShallow(apple, null);

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

4 Answers4

Linked

Related