Search code examples
javaarraylistclone

If clone( ) for ArrayList is broken why can't they fix it?


After reading multiple posts (here's one) about how clone( ) from the ArrayList class is broken, I was wondering no one has re-implemented it so that it isn't broken where people can be more comfortable using the method.

Is there something I'm missing here for a reason why no one has re-implemented it so that it is not broken?


Solution

  • The clone() itself is not broken: it's implemented well according to the specification of the clone() method. The problematic place is this specification itself. As it was already discussed in the linked question, there are several drawbacks, namely the necessity to do the unchecked cast, the potential problems which arise if your ArrayList is overridden and the necessity to know that the input List is actually an ArrayList. A good practice for any code requiring the ArrayList is to accept any List (or even Collection) implementation: this way your code becomes more flexible.

    As it was already noted, there's a better alternative: using

    ArrayList<Type> copy = new ArrayList<>(source);
    

    It's universal: it will work for any source collection and the result will be exactly ArrayList (not derived class). And you should not worry about the performance. According to the implementation it's about the same. See the clone() method code:

    public Object clone() {
        try {
            @SuppressWarnings("unchecked")
            ArrayList<E> v = (ArrayList<E>) super.clone();
            v.elementData = Arrays.copyOf(elementData, size);
            v.modCount = 0;
            return v;
        } catch (CloneNotSupportedException e) {
            // this shouldn't happen, since we are Cloneable
            throw new InternalError();
        }
    }
    

    So it makes a shallow copy (fast operation of comstant complexity as shallow size is constant and small), then makes single array copy and replaces the mod count. Let's check the constructor:

    public ArrayList(Collection<? extends E> c) {
        elementData = c.toArray();
        size = elementData.length;
        // c.toArray might (incorrectly) not return Object[] (see 6260652)
        if (elementData.getClass() != Object[].class)
            elementData = Arrays.copyOf(elementData, size, Object[].class);
    }
    

    It calls the toArray of original collection, uses its result as the internal array for itself and updates the size. It copies the array only in the case if original collection incorrectly returned typed array from the toArray method. What happens if the input collection is also ArrayList? Check the ArrayList.toArray:

    public Object[] toArray() {
        return Arrays.copyOf(elementData, size);
    }
    

    See, it's exactly the same operation as in clone() method. Thus in both cases (clone and copy-constructor) you have single Arrays.copyOf(elementData, size) call and small constant overhead.