How to properly override clone method?

Question

I need to implement a deep clone in one of my objects which has no superclass.

What is the best way to handle the checked CloneNotSupportedException thrown by the superclass (which is Object)?

A coworker advised me to handle it the following way:

@Override
    public MyObject clone()
    {
        MyObject foo;
        try
        {
            foo= (MyObject) super.clone();
        }
        catch (CloneNotSupportedException e)
        {
            throw new Error();
        }

        // Deep clone member fields here

        return foo;
    }

This seems like a good solution to me, but I wanted to throw it out to the SO community to see if there are any other insights I can include. Thanks!

Answer 1

Do you absolutely have to use clone? Most people agree that Java's clone is broken.

Josh Bloch on Design - Copy Constructor versus Cloning

If you've read the item about cloning in my book, especially if you read between the lines, you will know that I think clone is deeply broken. [...] It's a shame that Cloneable is broken, but it happens.

You may read more discussion on the topic in his book Effective Java 2nd Edition, Item 11: Override clone judiciously. He recommends instead to use a copy constructor or copy factory.

He went on to write pages of pages on how, if you feel you must, you should implement clone. But he closed with this:

Is all this complexities really necessary? Rarely. If you extend a class that implements Cloneable, you have little choice but to implement a well-behaved clone method. Otherwise, you are better off providing alternative means of object copying, or simply not providing the capability.

The emphasis was his, not mine.

---

Since you made it clear that you have little choice but to implement clone, here's what you can do in this case: make sure that MyObject extends java.lang.Object implements java.lang.Cloneable. If that's the case, then you can guarantee that you will NEVER catch a CloneNotSupportedException. Throwing AssertionError as some have suggested seems reasonable, but you can also add a comment that explains why the catch block will never be entered in this particular case.

---

Alternatively, as others have also suggested, you can perhaps implement clone without calling super.clone.

Answer 2

Sometimes it's more simple to implement a copy constructor:

public MyObject (MyObject toClone) {
}

It saves you the trouble of handling CloneNotSupportedException, works with final fields and you don't have to worry about the type to return.

Answer 3

Use serialization to make deep copies. This is not the quickest solution but it does not depend on the type.

Answer 4

There are two cases in which the CloneNotSupportedException will be thrown:

1. The class being cloned does not implemented Cloneable (assuming that the actual cloning eventually defers to Object's clone method). If the class you are writing this method in implements Cloneable, this will never happen (since any subclasses will inherit it appropriately).
2. The exception is explicitly thrown by an implementation - this is the recommended way to prevent clonability in a subclass when the superclass is Cloneable.

The latter case cannot occur in your class (as you're directly calling the superclass' method in the try block, even if invoked from a subclass calling super.clone()) and the former should not since your class clearly should implement Cloneable.

Basically, you should log the error for sure, but in this particular instance it will only happen if you mess up your class' definition. Thus treat it like a checked version of NullPointerException (or similar) - it will never be thrown if your code is functional.

---

In other situations you would need to be prepared for this eventuality - there is no guarantee that a given object is cloneable, so when catching the exception you should take appropriate action depending on this condition (continue with the existing object, take an alternative cloning strategy e.g. serialise-deserialise, throw an IllegalParameterException if your method requires the parameter by cloneable, etc. etc.).

Edit: Though overall I should point out that yes, clone() really is difficult to implement correctly and difficult for callers to know whether the return value will be what they want, doubly so when you consider deep vs shallow clones. It's often better just to avoid the whole thing entirely and use another mechanism.

Answer 5

The way your code works is pretty close to the "canonical" way to write it. I'd throw an AssertionError within the catch, though. It signals that that line should never be reached.

catch (CloneNotSupportedException e) {
    throw new AssertionError(e);
}

Answer 6

If your object has a reasonable constructor (perhaps a no-arg constructor, or one that takes immutable objects) then I would just forget about calling super:

  public MyObject clone() {
         MyObject foo = new MyObject(this.firstParameter, this.secondParameter);
         //deep clone fields
         return foo;
  }

You could do it, of course, with any fields and just be sure to deep clone them after construction (or before).

It's pretty rare to actually need what Object's clone method gives you.

Edit: (In response to comment and downvote), as the commenter hints, it would be a good idea to make this class final if you did this, so that subclasses don't depend on the super clone returning and instance of the subclass.

Answer 7

You can implement protected copy constructors like so:

/* This is a protected copy constructor for exclusive use by .clone() */
protected MyObject(MyObject that) {
    this.myFirstMember = that.getMyFirstMember(); //To clone primitive data
    this.mySecondMember = that.getMySecondMember().clone(); //To clone complex objects
    // etc
}

public MyObject clone() {
    return new MyObject(this);
}