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 StackOverflow community to see if there are any other insights I can include. Thanks!

Moscow

If you know that the parent class implements Cloneable then throwing an AssertionError rather than just a plain Error is a bit more expressive. – Global

Edit: I'd rather not use clone(), but the project is already based on it and it wouldn't be worth refactoring all the references at this point. – Moscow

Better to bite the bullet now rather than later (well, unless you are just about to hit production). – Sonasonant

We have a month and a half left on the schedule until this is completed. We can't justify the time. – Moscow

I think this solution is perfect. And don't feel bad about using clone. If you have a class that is used as a transport object and it contains only primitves and immutables like String and Enums, everything else than clone would be a complete waste of time for dogmatic reasons. Just keep in mind what clone does and what not! (no deep cloning) – Semipermeable

Seems nobody else even tried to implement the method, suggest return type Object for compliance with Object.clone() – Cerated

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.

Valleau

Unfortunately, the project is already written around using the clone method, otherwise I would absolutely not use it. I entirely agree with you that Java's implementation of clone is fakakta. – Moscow

If a class and all its superclasses call super.clone() within their clone methods, a subclass will generally only have to override clone() if it adds new fields whose contents would need to be cloned. If any superclass uses new rather than super.clone(), then all subclasses must override clone() whether or not they add any new fields. – Stichometry

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.

Armidaarmiger

Answer 3

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);
}

Elbrus

See @polygenelubricants' answer for why this might be a bad idea. – Prospectus

@KarlRichter I've read their answer. It does not say it's a bad idea, other than that Cloneable in general is a broken idea, as explained in Effective Java. The OP already expressed that they have to use Cloneable. So I have no idea how else my answer could be improved upon, except perhaps deleting it outright. – Elbrus

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 sub-classes 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. serialize-deserialize, 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.

Indusium

If an object exposes a public cloning method, any derived object which did not support it would violate the Liskov Substitution Principle. If a cloning method is protected, I would think that it would be better to shadow it with something other than a method returning the proper type, to prevent a subclass from even trying to call super.clone(). – Stichometry

Answer 5

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

Twitter

This was a great and obvious solution considering I was already using GSON. – Strongminded

Answer 6

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);
}

Phocis

This does not work most of the time. You can't call clone() on the object returned by getMySecondMember() unless it has a public clone method. – Valleau

Obviously, you need to implement this pattern on every object you want to be able to clone, or find another way to deep clone each member. – Phocis

Yes, that is a necessary requirement. – Valleau

Answer 7

public class MyObject implements Cloneable, Serializable{   

    @Override
    @SuppressWarnings(value = "unchecked")
    protected MyObject clone(){
        ObjectOutputStream oos = null;
        ObjectInputStream ois = null;
        try {
            ByteArrayOutputStream bOs = new ByteArrayOutputStream();
            oos = new ObjectOutputStream(bOs);
            oos.writeObject(this);
            ois = new ObjectInputStream(new ByteArrayInputStream(bOs.toByteArray()));
            return  (MyObject)ois.readObject();

        } catch (Exception e) {
            //Some seriouse error :< //
            return null;
        }finally {
            if (oos != null)
                try {
                    oos.close();
                } catch (IOException e) {

                }
            if (ois != null)
                try {
                    ois.close();
                } catch (IOException e) {

                }
        }
    }
}

Frisby

So you've just written it to the file system and have read the object back. Okay, is this the best method to handle clone? Can anyone from SO community comment on this approach? I guess this unnecessarily ties Cloning and Serialization - two entirely different concepts. I will wait to see what others have to say about this. – Crosscheck

Its not into the file system, it in main memory (ByteArrayOutputStream). for deeply nested objects its the solution thats works well. Especially if you don`t need it often, e.g in unit test. where performance is not the main goal. – Proscribe

Answer 8

As much as the most of the answers here are valid, I need to tell that your solution is also how the actual Java API developers do it. (Either Josh Bloch or Neal Gafter)

Here is an extract from openJDK, ArrayList class:

public Object clone() {
    try {
        ArrayList<?> v = (ArrayList<?>) super.clone();
        v.elementData = Arrays.copyOf(elementData, size);
        v.modCount = 0;
        return v;
    } catch (CloneNotSupportedException e) {
        // this shouldn't happen, since we are Cloneable
        throw new InternalError(e);
    }
}

As you have noticed and others mentioned, CloneNotSupportedException has almost no chance to be thrown if you declared that you implement the Cloneable interface.

Also, there is no need for you to override the method if you don't do anything new in the overridden method. You only need to override it when you need to do extra operations on the object or you need to make it public.

Ultimately, it is still best to avoid it and do it using some other way.

Tamas

Answer 9

Just because java's implementation of Cloneable is broken it doesn't mean you can't create one of your own.

If OP real purpose was to create a deep clone, i think that it is possible to create an interface like this:

public interface Cloneable<T> {
    public T getClone();
}

then use the prototype constructor mentioned before to implement it:

public class AClass implements Cloneable<AClass> {
    private int value;
    public AClass(int value) {
        this.vaue = value;
    }

    protected AClass(AClass p) {
        this(p.getValue());
    }

    public int getValue() {
        return value;
    }

    public AClass getClone() {
         return new AClass(this);
    }
}

and another class with an AClass object field:

public class BClass implements Cloneable<BClass> {
    private int value;
    private AClass a;

    public BClass(int value, AClass a) {
         this.value = value;
         this.a = a;
    }

    protected BClass(BClass p) {
        this(p.getValue(), p.getA().getClone());
    }

    public int getValue() {
        return value;
    }

    public AClass getA() {
        return a;
    }

    public BClass getClone() {
         return new BClass(this);
    }
}

In this way you can easely deep clone an object of class BClass without need for @SuppressWarnings or other gimmicky code.

Imperforate