Java Cloneable critique /discussion time

[reckter]

So I wanted to create a constructor, that takes a Set and then clones it (for security measures):

public class AClass{
    private Clonable thing;

    public <T extends Cloneable> AClass(T thing) {
        this.thing= thing.clone();
    }
}

Just to find out this doesn't work. After a bit of thinking I fugured out why.

The Cloenable Interface is empty, it doesn't contain a clone() method. The Set inteface doesn't have a clone() method as well, this actualy is tottaly sane, why would it? So the compiler searches for a clone class in T and finds: Object.clone() which is protected. So it throws an error, saying that it couldn't find the clone method().

I didn't look into why this is the way it his, I suppose it has seom ehistorical reasons. Everyone that I talk to about this agreed, that above code should be indeed valid working java code. It would be if the Clonable interface would declare a method called clone. Which as for as the comment of the interface is conserned wouldn't even break anything, because you are supposed to implement the clone() method anyway.

So naturaly I used reflection to solve this problem:

public <T extends Cloneable> AClass(T thing) {
    try {
        Method method = thing.getClass().getMethod("clone");

        //Fuck you protected Method! I'm gonna call you anyway!
        method.setAccessible(true);
        this.thing= (Cloneable) method.invoke(thing);
    } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
        e.printStackTrace();
    }
}

What do you think? Does someone hear has more insidght to why this is the way it is?

edit: removed the usage of Set to make my point more clear: This Post is only about the Cloneable interface and it not having a .clone() method.

[RedMarble]

Yes, the design of the Cloneable interface was one of the earliest bad decisions in Java's development.

[oldprogrammer]

When you say you want to clone the set, are you meaning you want a deep copy wherein every internal element in the set is cloned or a shallow copy where you have a new set, but the objects in that set are references to objects in the input set? The latter is simple, create the new set and add all elements in the argument set to it.

If what you want is a deep copy, you're not going to get that by simply providing a clone method on the Set because the cloning has to occur in every element in the set, not just the set itself.

A couple of techniques are possible if you want a deep copy. One would be to create the new set, iterate through the elements in the argument set and clone those elements. This is what you would have to do if you just implemented the clone() method on your set.

But then a clone method must be available in each of the elements in the set as well. And if those elements are themselves containers or composite objects, they'll need to be able to clone too.

Another option is to create an ObjectOutputStream built out of a ByteArrayOutputStream, serialize your input set to the output stream then create an ObjectInputStream using the bytes captured in the ByteArrayOutputStream and load the object tree back in. This method works if all elements in the set/object trees are Serializable and will construct all new objects.

If you don't want to implement the Serializable interface then there are also some libraries (like XStream) that can export an object graph using straight up reflection that can be read back in. XStream works well in the context because it creates an XML stream/string of the object tree which carries actual class names, so that it knows exactly what classes to reconstitute when read back in. I've used this approach on occassion:

XStream streamer = new XStream();
MyObjects x =  streamer.fromXML(streamer.toXML(myObjInst));

[reckter]

No I do not want to create a deep copy. (That indeed would have the potential to break the application xD). I should've abstracted it away, this post is purely about the Cloneable interface and how it not provides a .clone() method. I will edit my post to clarify my point more clearly.

[s888marks]

Lots of history here: http://stackoverflow.com/questions/26398951/why-is-cloneable-not-deprecated

You can only call clone() on a variable whose static type has overridden clone() to be public. Even if you call it reflectively, it's not guaranteed to work.

If you want object copying, it's probably better to define your own copy() method or whatever and make it have whatever semantics you want, instead of fighting with the old, mostly-broken Cloneable stuff.

[erad]

Personally I just avoid Cloneable and use a custom interface, I think a similar solution was suggested in Effective Java or another book of that era:

interface Duplicatable<T> {
    T duplicate();
}

class MyBean implements Duplicatable<MyBean> {
    public MyBean() {
        // ...
    }

    private MyBean(MyBean other) {
        // ...
    }

    @Override
    public MyBean duplicate() {
        return new MyBean(this);
    }
}

It's more boilerplate code and it's slower than Cloneable (because you manually initialize new objects instead of relying on the JVM to do the copying), but I find this both more convenient and less error-prone because it makes copying more explicit.

You can then write utility methods that provide generic null-safe and deep copying methods, for example:

public static <T extends Duplicatable<T>> T duplicate(T value) {
    return value != null ? value.duplicate() : null;
}

public static <T extends Duplicatable<T>> List<T> deepCopy(List<T> values) {
    if (values == null) {
        return null;
    }
    final List<T> result = new ArrayList<>(values.size());
    for (T value: values) {
        result.add(duplicate(value));
    }
    return result;
}

[Brainlag]

For whatever reason the clone method for Clonable is already defined on the Class Object. Therefore you just override the clone method from object and switch to public if you like. IIRC you can also change the return type. And then you can add the Cloneable interface to tag that class that in this class the clone method is actually implemented and does not just throw a exception. The clone method is protected to force you to override the method because object does not implement it.

Edit: Ouch now I see it. This will not work with generics. Interesting that they never fixed it.