4

I have a generic class that needs to be able to clone objects of the parameter type. A very simple example is below. The compiler claims clone() from the type Object is not visible.

public class GenericTest<T extends Cloneable>
    {
    T obj;
    GenericTest(T t)
        {
        obj = t;
        }
    T getClone()
        {
        // "The method clone() from the type Object is not visible."
        return (T) obj.clone();
        }
    }

I'd prefer not to have the caller do the cloning since there are other things that have to happen to maintain the integrity of the object. The code above is just an illustration of the problem without the noise of the other data I have to maintain related to the cloned object.

Is there a way around this or is this another one of those cases where the designers of Java consider rationalizing its shortcomings the equivalent of having none?

Craig
  • 3,253
  • 5
  • 29
  • 43
  • 2
    Craig - I don't think the generics contribute to your problem here, it's more or less down to the fact that `Object.clone()` is protected. See [this question](http://stackoverflow.com/questions/1138769/why-is-the-clone-method-protected-in-java-lang-object) for more context. – Andrzej Doyle Mar 22 '11 at 16:50
  • Are you implementing clone on the class T that you are passing in? Cloneable is implemented in Object, but to work properly it has to be implemented on each derived class that you are using as well. – liam Mar 22 '11 at 16:51

3 Answers3

6

Because the method is marked as protected on the Object class, you cannot in general call this method on arbitrary objects. Personally I didn't think this would be a problem at first (hey, I'm a subclass of Object, so I should be able to call its protected methods, right?), but the compiler needs to know that you're a subclass of the target object's class (or in its package) in order to call protected methods, neither of which apply here.

The idea behind the clone() method is that classes which supported it would override the method, declaring it as public.

The only real solution here that preserves full functionality is to use reflection to access the method and get around the access modifiers. An alternative would be to write your own MyCloneable interface which has a public clone() method declared on it; this might work if you'll only ever be passing your own domain classes in, but means that you couldn't use it on external classes (such as java.util.String or java.util.ArrayList) since you can't force them to implement your interface.

As per answers to the linked question, this is a very dubious design.

Community
  • 1
  • 1
Andrzej Doyle
  • 102,507
  • 33
  • 189
  • 228
3

A mistake on Java's part. Reflection is the right way to go

static Method clone = Object.class.getMethod("clone"); 

static public <T extends Cloneable> 
T clone(T obj)
    return (T) clone.invoke(obj);
irreputable
  • 44,725
  • 9
  • 65
  • 93
  • Of course, not every object implementing the Cloneable interface has to be in fact cloneable ... this is only a marker interface saying to the implementation in Object "please don't throw an exception". – Paŭlo Ebermann Mar 22 '11 at 23:22
  • ... and conversely, not every object that is cloneable needs to implement the `Cloneable` interface -- if it doesn't use `Object.clone()`, then it does not need to care about `Cloneable` – newacct Mar 23 '11 at 04:35
  • Just because Java didn't make an interface that indicates whether a class can be cloned (i.e. has a public method for cloning) does not mean that it's a "mistake" – newacct Mar 23 '11 at 04:44
  • 1
    @newacct It is a mistake: Read the comments at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4098033 – Max Nanasy Jun 13 '13 at 01:57
  • @MaxNanasy: I don't need to read what someone else said. We can reason about it right here. – newacct Jun 13 '13 at 02:31
  • 1
    @newacct The reason I linked to that bug is because Java implementers Joshua Bloch and Ken Arnold (although they're not necessarily the original designers of Cloneable) indicate that the current Cloneable interface is "practically useless" and that they would add the public clone method if it weren't an incompatible change, although I realize upon further reflection that, since they were probably not the original Cloneable designers, this does not represent an open-and-shut case of an admission of mistake. – Max Nanasy Jun 13 '13 at 06:11
  • @MaxNanasy: "useless" does not mean "mistake". "useless" just means that it is not very useful. Lot of things in many languages are not very useful. "mistake" means there is something wrong, incorrect, violating the specification. I do not disagree that it would be nice to have another interface that specified a method for cloning. But just because the Java library doesn't provide this does not mean it is incorrect. – newacct Jun 13 '13 at 23:34
  • 1
    @newacct I interpreted irreputable's "A mistake on Java's part" as indicating that it was a mistake made in the design of the Java programming language, and I would consider not making `clone()` a member of `Cloneable` a language design mistake. – Max Nanasy Jun 14 '13 at 03:13
  • @MaxNanasy: I do not consider there to be any mistakes. You think that way because you have an incorrect notion of what `Cloneable` is. `Cloneable` is defined as "does not throw exception when `Object.clone()` is called on this". That's it. It is not for "a type which has a clone method". Maybe the naming is not clear. – newacct Jun 14 '13 at 07:28
  • 4
    @newacct The fact that `Cloneable` is not defined as "a type which has a clone method" is the design mistake (IMO), since it would have been more intuitive and useful if it had been defined as such. – Max Nanasy Jun 14 '13 at 10:43
  • Not sure if Java has changed since this answer was posted and accepted, but it doesn't seem to work now. Object.class.getMethod only returns public methods and returns null for "clone". Instead use Object.class.getDefinedMethod("clone"), and call setAccessible(true) on the returned method object – Dale Feb 01 '18 at 01:26
0

I needed to clone a POJO with fields in it. What did work was the following:

public static Object clone(Object o)
{
  Object clone = null;

  try
  {
     clone = o.getClass().newInstance();
  }
  catch (InstantiationException e)
  {
     e.printStackTrace();
  }
  catch (IllegalAccessException e)
  {
     e.printStackTrace();
  }

  // Walk up the superclass hierarchy
  for (Class obj = o.getClass();
    !obj.equals(Object.class);
    obj = obj.getSuperclass())
  {
    Field[] fields = obj.getDeclaredFields();
    for (int i = 0; i < fields.length; i++)
    {
      fields[i].setAccessible(true);
      try
      {
        // for each class/suerclass, copy all fields
        // from this object to the clone
        fields[i].set(clone, fields[i].get(o));
      }
      catch (IllegalArgumentException e){}
      catch (IllegalAccessException e){}
    }
  }
  return clone;
}
Skip
  • 6,240
  • 11
  • 67
  • 117