The correct way to deep copy using copy constructor instead of Object.clone

Question

I have code that executes a deep copy using Object.clone, but I'm trying to rewrite it using a more "acceptable" instance constructor method. Below are two simple examples of what I'm trying to do, the first using cloning and the second using the copy constructor.

Deep copy using a clone

import java.util.*; abstract class Person implements Cloneable { String name; public Object clone() throws CloneNotSupportedException { return super.clone(); } } class Teacher extends Person implements Cloneable { int courses; public String toString() { return name + ": courses=" + courses; } } class Student extends Person implements Cloneable { double gpa; public String toString() { return name + ": gpa=" + gpa; } } public class DeepCopy_Clone { private static List<Person> deepCopy(List<Person> people) throws CloneNotSupportedException { List<Person> copy = new ArrayList<Person>(); for (Person person : people) { copy.add((Person)person.clone()); } return copy; } public static void main(String[] args) throws CloneNotSupportedException { ArrayList<Person> people = new ArrayList<Person>(); Teacher teacher = new Teacher(); teacher.name = "Teacher"; teacher.courses = 5; people.add(teacher); Student student = new Student(); student.name = "Student"; student.gpa = 4.0; people.add(student); List<Person> peopleCopy = deepCopy(people); // Invalidate the original data to prove a deep copy occurred teacher.name = null; teacher.courses = -1; student.name = null; student.gpa = -1; for (Person person : peopleCopy) { System.out.println(person.toString()); } } } 

Deep copy using copy constructor

  import java.util.*; abstract class Person { String name; public Person() {} public Person(Person other) { this.name = other.name; } public Person deepCopy() { if (this instanceof Teacher) { return new Teacher((Teacher)this); } else if (this instanceof Student) { return new Student((Student)this); } throw new Error("Unknown type of person"); } } class Teacher extends Person { int courses; public Teacher() {} public Teacher(Teacher other) { super(other); this.courses = other.courses; } public String toString() { return name + ": courses=" + courses; } } class Student extends Person { double gpa; public Student() {} public Student(Student other) { super(other); this.gpa = other.gpa; } public String toString() { return name + ": gpa=" + gpa; } } public class DeepCopy_ConstructorAlternative { private static List<Person> deepCopy(List<Person> people) { List<Person> copy = new ArrayList<Person>(); for (Person person : people) { copy.add(person.deepCopy()); } return copy; } public static void main(String[] args) { ArrayList<Person> people = new ArrayList<Person>(); Teacher teacher = new Teacher(); teacher.name = "Teacher"; teacher.courses = 5; people.add(teacher); Student student = new Student(); student.name = "Student"; student.gpa = 4.0; people.add(student); List<Person> peopleCopy = deepCopy(people); // Invalidate the original data to prove a deep copy occurred teacher.name = null; teacher.courses = -1; student.name = null; student.gpa = -1; for (Person person : peopleCopy) { System.out.println(person.toString()); } } } 

Interestingly, despite all the talk about the evil of cloning in Java, the clone alternative requires less code and fewer throws (at least in this particular case).

I will be grateful for feedback on the copy constructor alternative. Could you do otherwise? Thanks.

Answer 1

Instead:

  public Object clone() throws CloneNotSupportedException { return super.clone(); } 

I would prefer:

 public Person clone() { try { return (Person) clone(); } catch (CloneNotSupportedException e) { throw new RuntimeException("This should be impossible ..."); } } 

therefore, callers do not need to handle an exception that can never occur, and it does not need to be thrown.

In a copy-constructor approach, type switching is better processed polymorphically:

 abstract class Person { ... public abstract Person deepCopy(); } class Student { ... public Student deepCopy() { return new Student(this); } } class Teacher { ... public Teacher deepCopy() { return new Teacher(this); } } 

now the compiler can verify that you have provided a deep copy for all subtypes and that you do not need any casts.

Finally, note that the cloning and copy constructor approach has the same open api (regardless of whether the clone() or deepCopy() method is called), so the approach you use is an implementation detail. The copier-constructor approach is more detailed, since you provide both a constructor and a method that calls this constructor, but it is easier to generalize to a general type conversion tool, allowing things like:

 public Teacher(Person p) { ... say("Yay, I got a job"); } 

Recommendation: use a clone if only an identical copy is required, use copy constructors if your caller can request an instance of a certain type.

Answer 2

Note that in the Person.deepCopy approach of the copy constructor, the Person class must check all its subclasses explicitly. This is a fundamental design, a problem of code maintenance and testing: this will prevent successful cloning if someone introduces a new subclass of Person , having forgotten or cannot update Person.deepCopy . The .clone() method avoids this problem by providing a virtual method ( clone ).

Answer 3

One of the advantages of the clone-based approach is that, when properly implemented, derived types that themselves do not require special behavior when cloning does not require a special clone code. By the way, I tend to think that classes that expose the cloning method are usually not inherited; instead, the base class must support cloning as a protected method, and the derived class must support cloning through an interface. If an object does not support cloning, it should not throw an exception from the Clone API; instead, the object should not have a clone API.