Cloning objects in Java [3 questions]

Question

will the clone method of Asub be called by doing this? Or is Asub deep cloned properly? If not, is there a way to propery deep clone Asub through this kind of method?

abstract class Top extends TopMost {
    protected Object clone() {
        Object obj = super.clone();
        // deep copy and try catch
    }


}

abstract class A extends Top { 
    protected Object clone() {
        Object obj = super.clone();
       // deep copy and try catch
    } 


}

class Asub extends A {
    protected Object clone() {
        Object obj = super.clone();
        // deep copy and try catch
    }

    public void doSomethingNew() {
    }
}

abstract class TopMost {
    public void someMethod() {
        Top a = (Top) super.clone();
        // more code here
    }
}

public class Main {
    public static void main(String... args) {
        Asub class1 = new Asub();
        class1.someMethod();
    }
}

Answer 1

First of all, note that the clone() interface is broken, thus should not be used in new code. It is better to implement copy constructor(s) instead.

However, if you really need to do it, the proper way is for TopMost to implement Cloneable. Why? Says Effective Java 2nd Edition, Item 11:

So what does Cloneable do, given that it contains no methods? It determines the behavior of Object’s protected clone implementation: if a class implements Cloneable, Object’s clone method returns a field-by-field copy of the object; otherwise it throws CloneNotSupportedException.This is a highly atypical use of interfaces and not one to be emulated. Normally, implementing an interface says something about what a class can do for its clients. In the case of Cloneable, it modifies the behavior of a protected method on a superclass.

Moreover, Asub.clone should be declared public, not protected - otherwise you can't call it from the outside world. Also, if you are using Java5 or above, it is legal and desirable for Asub.clone to return Asub, not Object (and similarly for its superclasses).

You don't show any members in the classes - the implementations of clone in the various classes can be a whole lot different depending on the types of members in that class. Namely, if a class has any mutable members, you need to carefully deep copy all of them, otherwise you end up with distinct objects sharing their internal state.

However, supposing your classes have only primitive or immutable fields, the cloning works as expected, although you have a lot of unnecessary clone methods in your abstract classes, all of which simply call super.clone() - you may be better off with Asub.clone() only.

As a side note, if Top a = (Top) super.clone() is not a typo, you introduce a dependency from base class to derived class, which is not a good idea.

Answer 2

The call to super.clone() disables the virtual mechanism, so it only calls Object.clone()

Answer 3

By allowing all abstract subclasses implementing super.clone() essentially does nothing (since all your abstract classes in your example are doing nothing) and just call (at the end) Object.clone() method.

My suggestion is to allow all concrete classes (like ASub) to override the clone method and use the copy constructor idiom to create an exact clone of itself....

e.g.

public abstract class TopMost {

    public TopMost(TopMost rhs) {

    }

}

public abstract class Top extends TopMost {

    public Top(Top rhs) {
        super(rhs);

        //whatever you need from rhs that only is visible from top
    }
}

public abstract class A extends Top { 

    public A (A rhs) {
        super(rhs);

        //TODO: do rhs copy
    }
}

public class ASub extends A {

    public ASub(ASub rhs) {
        super(rhs);

        //TODO: copy other stuff here....
    }

    public Object clone() {
        return new ASub(this);
    }
}

PS Make TopMost Cloneable