8

Right now, I have:

    public <T> T[] toArray(T[] old) {
        T[] arr = Arrays.copyOf(old, old.length + size());
        int i = old.length;
        for(E obj : this) {
            arr[i] = old.getClass().getComponentType().cast(obj);
            ++i;
        }
        return arr;
    }

(Note that this does not follow the contract as it was pointed out by axtavt.)

where I get this warning:

Type safety: Unchecked cast from capture#2-of ? to T

Is this still the best / most straightforward way to implement it? Can I somehow code it in a way without that warning? How would I implement it otherwise?


Edit: My current solution. First, I really wanted to not have such a warning in toArray itself. Therefore, I coded these little helper functions (read here for a further discussion about these):

@SuppressWarnings("unchecked") static <T> Class<? extends T> classOf(T obj) {
    return (Class<? extends T>) obj.getClass();
}

@SuppressWarnings("unchecked") static <T> Class<? extends T> classOf(T[] array) {
    return (Class<? extends T>) array.getClass().getComponentType();
}

@SuppressWarnings("unchecked") static <T> T[] newArray(Class<T> clazz, int size) {
    return (T[]) Array.newInstance(clazz, size);
}   

Now, my toArray implementation looks like:

    public <T> T[] toArray(T[] array) { 
        int size = size();
        if (array.length < size) { 
            array = newArray(classOf(array), size);
        } else if (array.length > size) {
            array[size] = null;
        }

        int i = 0;
        for (E e : this) {
            array[i] = classOf(array).cast(e);
            i++;
        }
        return array;
    } 
2
  • 1
    You want a Collection<T> in the signature somewhere?
    – user166390
    Oct 24, 2010 at 23:08
  • All you can do is cast to (T), the cast() method only changes the type of the reference, it won't change the type of the object referenced. Oct 24, 2010 at 23:41

2 Answers 2

8

Is this still the best / most straightforward way to implement it? How would I implement it otherwise?

It's not how Josh Bloch did. Have a look at the source of AbstractCollection#toArray(). Here's an extract of relevance from JDK 1.6.0_22.

public <T> T[] toArray(T[] a) {
    // Estimate size of array; be prepared to see more or fewer elements
    int size = size();
    T[] r = a.length >= size 
        ? a 
        : (T[]) Array.newInstance(a.getClass().getComponentType(), size);
    Iterator<E> it = iterator();

    for (int i = 0; i < r.length; i++) {
        if (!it.hasNext()) { // fewer elements than expected
            if (a != r)
                return Arrays.copyOf(r, i);
            r[i] = null; // null-terminate
            return r;
        }
        r[i] = (T) it.next();
    }
    return it.hasNext() ? finishToArray(r, it) : r;
}

The source code is available in src.zip file of the JDK. You can integrate it in any decent IDE like Eclipse/IDEA/Netbeans so that you can see it when you open the AbstractCollection class.

Can I somehow code it in a way without that warning?

No. Use @SuppressWarnings("unchecked") if it is bothering you.

That said, I'd suggest to extend AbstractCollection instead of implementing Collection if possible so that you have at least the basic features already implemented for you.

0
5

First of all, if it's supposed to be an implementation of Collection.toArray(), it doesn't follow the contract - you shouldn't keep old elements in the array (see javadoc).

The correct implementation looks like this:

public <T> T[] toArray(T[] array) { 
    int size = size();
    if (array.length < size) { 
        // If array is too small, allocate the new one with the same component type
        array = Array.newInstance(array.getClass().getComponentType(), size);
    } else if (array.length > size) {
        // If array is to large, set the first unassigned element to null
        array[size] = null;
    }

    int i = 0;
    for (E e: this) {
        // No need for checked cast - ArrayStoreException will be thrown 
        // if types are incompatible, just as required
        array[i] = (T) e;
        i++;
    }
    return array;
} 
1
  • Ah thanks for the hint. Because of this, I didn't had the Javadoc available and was too lazy to search for it online. :)
    – Albert
    Oct 24, 2010 at 23:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.