8

I'm reading on Joshua Bloch's Effective Java, 2nd edition, Item 11: Override clone judiciously.

On page 56, he is trying to explain that when we override clone() for some classes (like collection classes), we must copy the internals of it. He then gives the example of designing a class Stack:

public class Stack {
    private Object[] elements;
    private int size = 0;
    private static final int DEFAULT_INITIAL_CAPACITY = 16;
    public Stack() {...}
    public void push(Object e) {...}
    public Object pop() {...}
    private void ensureCapacity() {...} //omitted for simplicity
}

He claims that if we simply use super.clone() to clone a Stack, the resulting Stack instance "will have the correct value in its size field, but its elements field will refer to the same array as the original Stack instance. Modifying the original will destroy the invariants in the clone and vice versa. You will quickly find that your program produces nonsensical results or throws a NullPointerException." Now that seems fair. But he then gives an example of the "correct implementation", which confuses me:

@Override public Stack clone() {
    try {
        Stack result = (Stack) super.clone();
        result.elements = elements.clone();
        return result;
    } catch (CloneNotSupportedException e) {
        throw new AssertionError();
    }
}

Now how is that different from super.clone()? I know, the new Stack.element will be a different reference than the old one and all; but the "internals" of the array are still the same, aren't they? The actual elements of the array result.element still point to the original Object references. That could still result in destroying the invariants of the clone when changing the original, or vice versa, couldn't it? Am I missing anything?

justgivememyicecream
  • 260
  • 1
  • 11
  • You can find [here](https://stackoverflow.com/questions/5821851/does-calling-clone-on-an-array-also-clone-its-contents) that what you said is correct. `elements.clone()` will not clone the content. – AxelH Jul 25 '18 at 10:02

4 Answers4

5

Now how is that different from super.clone()?

Because the arrays are now different. If two Stacks share the same array then, when one adds or removes from the stack, the size field in the other Stack is not updated, leading to discrepancies.

The array's objects are not cloned themselves. This is deliberate as they do not need to be cloned. It is expected that two Stacks - or indeed any two Collections - can contain references to the same objects. You would get the same behaviour with this code:

Foo foo = new Foo()
Stack stackOne = new Stack();
Stack stackTwo = new Stack();
stackOne.push(foo);
stackTwo.push(foo);

It's not inherently a problem, and is usually the desirable behaviour.

Michael
  • 41,989
  • 11
  • 82
  • 128
  • Thanks. What I was failing to understand was that when both arrays contain references that point to the same group of objects, and one of them deletes one element, the other array (whose `size` did not change) will still hold the reference to that *deleted* element and hence its invariant persists. – justgivememyicecream Jul 26 '18 at 01:18
2

You are absolutely right about how clone works. The objects in the backing array will not be copied, but the backing array will be copied.

That is not a problem because the caller is not expecting the elements to be copied anyway. For collection classes like stacks, the "norm" is to do a shallow copy. One example from the standard library is the copy constructor of ArrayList.

Also note that you could implement clone by cloning the objects inside the array as well (this would mean that the stack can only store Clonable objects that expose clone). That would not break the contract of clone. The contract is very loose.

Sweeper
  • 213,210
  • 22
  • 193
  • 313
  • Presumably if you did want to clone each object, the variable type would have to be one that implements `Clonable` and also exposes the `clone()` method. You can't call `clone()` on an `Object` reference. – khelwood Jul 25 '18 at 10:24
  • 2
    Btw "implements Cloneable" and "exposes the clone() method" are two separate requirements. You can't call `clone()` on a reference of type `Cloneable` either. – khelwood Jul 25 '18 at 10:29
  • Good point about the copy constructor of `ArrayList`. And thanks for giving the idea of the "norm" way of cloning Collections. – justgivememyicecream Jul 26 '18 at 01:22
1

Modifying the original will destroy the invariants in the clone and vice versa.

The problem is that modifying one stack (pushing new elements into it, or removing them) would modify both stacks, if they were to share the same backing array, but not consistently - for instance, the size member would be updated in one but not the other. If one of the invariants is for instance that no elements in the array past the current "stack top" are not null, that invariant could be broken. (I don't however think this could directly result in exceptions, for this particular case).

By cloning the array, the two stacks have separate arrays (which contain the same elements), as you've surmised. Pushing an element into one stack then won't affect the contents of the array which backs the other stack.

The actual elements of the array result.element still point to the original Object references. That could still result in destroying the invariants of the clone when changing the original, or vice versa, couldn't it? Am I missing anything?

No, it couldn't. (Try to come up with an example of how this could happen; you'll find yourself stumped). The invariants of the stack class depend on the array size and contents (identity), not the state of the objects that are in the array.

davmac
  • 20,150
  • 1
  • 40
  • 68
1

You can not do anything more than that. super.clone() copies value members, elements.clone() creates a new array so the new stack will have independent storage from the old one, and that is all, for the stack construct itself.
Objects on the stack are not necessarily cloneable, so it is a matter of decision to attempt cloning or not cloning them one by one, and chances are that this is addressed somewhere in the text. (Side note: built-in containers do not clone objects when you addAll(), putAll(), or use a constructor accepting another container)

tevemadar
  • 12,389
  • 3
  • 21
  • 49