8

Is it a good practice to override clone method, without implementing Cloneable interface and not calling super.clone(). This way, CloneNotSupportedException exception will not be thrown.

Consider this class :

class Money {

    private BigDecimal x ;

    public Object clone() {
        Money m1 = new Money();
        m1.setX(this.x);
        return m1;
    }

    public BigDecimal getX() {
        return x;
    }

    public void setX(BigDecimal x) {
        this.x = x;
    }

}        

This class, does not throw CloneNotSupportedException and it works just like a copy constructor.

Is this a good way to do it ?

Rinke
  • 6,095
  • 4
  • 38
  • 55
Vinoth Kumar C M
  • 10,378
  • 28
  • 89
  • 130

6 Answers6

5

You have your cloning logic and of course you know, that your class supports cloning and therefore clone method should not throw CloneNotSupportedException. So calling super.clone() here causes you to write boilerplate try/catch block with rethrowing CloneNotSupportedException wrapped inside AssertionError, which is obviously never thrown. And this marker Cloneable... I think, that this part of Java is misdesigned. So I just ignore documentation and copy fields by hands.

The only two arguments for using super.clone() are performance (I suppose, something like memcpy used internally) and persistence to errors when new fields added.

Alexey Andreev
  • 1,980
  • 2
  • 17
  • 29
4

It's bad practice not to call super.clone(). See this answer for more on how to implement the clone method.

Community
  • 1
  • 1
Rinke
  • 6,095
  • 4
  • 38
  • 55
2

In your case, it's workable, but in some cases, it doesn't even work. Implementing a clone method without calling super.clone would make it impossible to make the subclass cloneable. For example, suppose I have a Fruit class

public class Fruit implements Cloneable {

    private String fruitName;

    public Fruit(String fruitName) {
        this.fruitName = fruitName;
    }

    public String getFruitName() {
        return fruitName;
    }

    public Fruit clone() {
        return new Fruit(fruitName);
    }
}

As you can see, I implement Fruit#clone method without calling super.clone, this seems fine. But now I want to create a class Apple which extends Fruit, like this:

public class Apple extends Fruit {

    private String origin;

    public Apple(String fruitName, String origin) {
        super(fruitName);
        this.origin = origin;
    }

    public Apple clone() {
        // TODO
    }

}

Now the problem is, how to implement Apple's clone method. Should I call super.clone inside Apple's clone method? Yes, you should, otherwise you cannot get the value of fruitName, which is a private field inside Fruit. OK, let's try this:

public Apple clone() {
    return (Apple) super.clone();
}

However, it didn't work. Because Fruit#clone doesn't call super.clone, so the result of super.clone() inside Apple#clone returns an instance of Fruit instead of Apple. Fruit cannot be cast into Apple, thus an error would be thrown:

Exception in thread "main" java.lang.ClassCastException: Fruit cannot be cast to Apple
    at Apple.clone(Apple.java:20)

So as you can see, you cannot provide a workable clone method for Apple, whether you call super.clone or not, it wouldn't work anyway. It's just because you didn't call super.clone in Fruit#clone.

In conclusion, you need to call super.clone in the clone method if you want the subclass to be cloneable.

Searene
  • 25,920
  • 39
  • 129
  • 186
0

In your case you have a class which has the clone method without telling the outside world that it is actually Cloneable. This is not called cloning but prototyping.

If you want to clone use the interface if not choose some other method name.

Please check the documentation as well:

Creates and returns a copy of this object. The precise meaning of "copy" may depend on the class of the object. The general intent is that, for any object x, the expression: x.clone() != x will be true, and that the expression: x.clone().getClass() == x.getClass() will be true, but these are not absolute requirements. While it is typically the case that: x.clone().equals(x) will be true, this is not an absolute requirement. By convention, the returned object should be obtained by calling super.clone. If a class and all of its superclasses (except Object) obey this convention, it will be the case that x.clone().getClass() == x.getClass().

There is a reason you have to call super.clone().

Adam Arold
  • 29,285
  • 22
  • 112
  • 207
  • 1
    Your quote confirms that OP's implementation does not break the contract. – Marko Topolnik Dec 02 '13 at 09:54
  • **By convention, the returned object should be obtained by calling super.clone** – Adam Arold Dec 02 '13 at 09:54
  • What happens if someone wants to actually override `clone`? – Adam Arold Dec 02 '13 at 09:55
  • Do you understand the meaning of the term *should*? Hint: it is different from *must*. – Marko Topolnik Dec 02 '13 at 09:55
  • I think most java conventions are "must". If you don't follow them others get confused. At least he shouldn't name it `clone` but `copy` like others commented. – Adam Arold Dec 02 '13 at 09:58
  • 1
    My point was, OP is not breaking the contract. He is straying from convention, but that's a separate point. If his class is `final`, for example, it cannot even be determined from the outside whether or not he is relying on `super.clone()`. Nobody gets confused by invisible implementation details. – Marko Topolnik Dec 02 '13 at 10:00
  • I see. What is your suggestion? How he should solve this problem? – Adam Arold Dec 02 '13 at 10:01
  • 1
    I don't see any problem in the question. He is asking whether "it is a good way to do it". So my answer would be "if you plan to make your class `final`, then it's perfectly OK." – Marko Topolnik Dec 02 '13 at 10:03
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/42315/discussion-between-adam-arold-and-marko-topolnik) – Adam Arold Dec 02 '13 at 10:04
0

clone is all about the coping/duplicating the type and state of an object. And if we have to identify the type and it make sense to have it associated with the Object class. Java by nature, operates by reference and by principle of shallow copy. If we have to enable deep copy then user has to indicate the runtime special handling is been done in clone(). And that is done by tagging with the Clonable interface.

Pradeep Kr Kaushal
  • 1,506
  • 1
  • 16
  • 29
0

theoretically adding an override of clone is the right way to enforce the prototype pattern, i.e. creating copies of an object at runtime without knowing the type.

Practically its a different story. Clone is the API standard way to do it. When you are writing your own clone method for a standalone class its ok to drop the super.clone call and go ahead. But its a naive practice. Assume that you have a class which follows an inheritance mechanism. Imagine, a Lion which is a Carnivore and thus in turn is an Animal. Now if you drop the super.clone in Lion you could end up recreating a copy of Lion without it being a Carnivore or god forbid even an Animal.

So its best to follow the API standards and keep yourself free of any hickups.

Nazgul
  • 1,892
  • 1
  • 11
  • 15