7

I'm writing a class in which I have to override the clone() method with the infamous "super.clone() strategy" (it's not my choice).

My code looks like this:

@Override
public myInterface clone()
{
    myClass x;
    try
    {
        x = (myClass) super.clone();
        x.row = this.row;
        x.col = this.col;
        x.color = this.color; 
        //color is a final variable, here's the error
    }
    catch(Exception e)
    {
        //not doing anything but there has to be the catch block
        //because of Cloneable issues
    }
    return x;
}

Everything would be fine, except that I can't initialize color without using a constructor, being it a final variable... is there some way to both use super.clone() AND copying final variables?

  • 1
    Isn't that a bit redundant, since you're using `super.clone();` already? – Kayaman Dec 19 '15 at 21:46
  • 2
    @Kayaman `myclass` might be derived from `someClass`, which (apart from all `someClass` members), also has `row`, `col` and `color`. So `super.clone()` won't copy such members. – Zereges Dec 19 '15 at 21:47
  • @Zereges I wasn't asking for speculation, I was asking for facts. – Kayaman Dec 19 '15 at 21:48
  • @Kayaman It's probable, that OP wants to copy members, which are copied by `super.clone()` – Zereges Dec 19 '15 at 21:49
  • @Zereges To me it's more probable that he doesn't fully understand how `clone()` works (which isn't surprising). That's why I didn't want speculation. Facts work better than guesses on SO. – Kayaman Dec 19 '15 at 21:57
  • My class implements an interface, which doesn't have `row`, `col` and `color`, just as @Zereges said. –  Dec 19 '15 at 22:01
  • 1
    @JohnStrife Zereges never said that. The call to `super.clone()` will copy all the fields (otherwise what would be the advantage of calling it?), so the copying by hand is unnecessary. You can replace the whole method with `return (MyInterface)super.clone();`. – Kayaman Dec 19 '15 at 22:07
  • Jeez, it's like herding cats. – Kayaman Dec 19 '15 at 22:10
  • Will the super.clone() also copy the fields that are defined only in `myClass`, and not in the class it's derived from (which I guess being Object)? –  Dec 19 '15 at 22:11

3 Answers3

2

Since the call to super.clone(); will already create a (shallow) copy of all the fields, final or not, your full method will become:

@Override
public MyInterface clone() throws CloneNotSupportedException {
    return (MyInterface)super.clone();
}

This requires that the superclass also implements clone() properly (to ensure that the super.clone() eventually reaches the Object class. All the fields will be copied properly (including final ones), and if you don't require deep clones or any other special functionality, you can use this and then promise that you'll never try to implement clone() again (one of the reasons being that it's not easy to implement it correctly, as evident from this question).

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • 1
    The wording from [the documentation](http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#clone()): "The method clone for class Object performs a specific cloning operation. First, if the class of this object does not implement the interface Cloneable, then a CloneNotSupportedException is thrown. ... Otherwise, this method creates a new instance of the class of this object and initializes **all** its fields with exactly the contents of the corresponding fields of this object, as if by assignment" (emphasis mine) – augray Dec 19 '15 at 22:17
  • Well, I guess this class derives directly from Object, so I don't know if it's `clone() ` is good... edit: I understood now. Thanks. –  Dec 19 '15 at 22:18
  • @JohnStrife Well I know. It is (however `Object` does not implement `Cloneable`, another "funny" thing about `clone()`). As for augray, I don't know what your point was, but does it really surprise you if a method called `clone()` creates a clone of an object? – Kayaman Dec 19 '15 at 22:20
  • 1
    Now I hope everyone learned a lesson why using `clone()` is a bad idea, and that there are plenty of [better ways](http://stackoverflow.com/questions/2156120/java-recommended-solution-for-deep-cloning-copying-an-instance) to do it, especially when dealing with complex objects and mutable object properties. – Kayaman Dec 19 '15 at 22:35
  • Yeah, I had already read from many sources that using the clone() method is a mess, I just had no choice this time. –  Dec 19 '15 at 22:50
1

Under the assumption that you don't have another choice you can use Reflection. The following code shows how this forks for the field color. You need some try-catch around it.

Field f =  getClass().getDeclaredField("color");
f.setAccessible(true);
f.set(x, this.color)
Denis Lukenich
  • 3,084
  • 1
  • 20
  • 38
1

One thing to keep being mindful of, when mixing clone and final fields, especially the one with initializers, is that (as one of the comments rightfully says), the values are copied from the original, no matter whether that field is final or not, and initializer be damned.

So if a final field has a dynamic initializer, it's actually not executed. Which, IMHO, is breaking a serious expectation that if I have a final instance field in a class, every new instance of this class is going to initialize the field to whatever value the initializer says it should be.

Here is an example where this can be a (insert your level of seriousness) problem:


import lombok.SneakyThrows;
import java.util.HashMap;
import java.util.Map;

public class App {

    public static void main(String[] a) {

        Cloned c1 = new Cloned();
        c1.d.put("x", "x");
        Cloned c2 = c1.clone();
        System.out.println("c2.x="+c2.d.get("x"));
        c1.d.clear();
        System.out.println("c2.x="+c2.d.get("x"));

    }

    static class Cloned implements Cloneable {

        public final Map<String, String> d = new HashMap<>();

        @Override
        @SneakyThrows
        public Cloned clone() {
            return (Cloned) super.clone();
        }
    }

}

Output:

c2.x=x
c2.x=null
Pawel Veselov
  • 3,996
  • 7
  • 44
  • 62