A custom Java collection which doesn't support nulls

Question

I am making a GWT application and have to prevent client code from using null values in collections. I found an answer but GWT doesn't support Queue and its implementations as the GWT documentation says.

So, I had to make my own implementation.

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;

/**
 * This implementation of {@link ArrayList} does not allow to add null values
 * 
 * @author Maxim Dmitriev
 * 
 * @param <E>
 */
public class NonNullArrayList<E> extends ArrayList<E> implements Serializable {

    private static final String NULL_VALUES_ARE_PROHIBITED = "Null values are prohibited";
    private static final String CLONING_IS_UNSUPPORTED = "Cloning is unsupported. Please support if necessary";

    public NonNullArrayList() {
        // For serialization. super() is called automatically.
    }

    public NonNullArrayList(int initialCapacity) {
        super(initialCapacity);
    }

    public NonNullArrayList(Collection<? extends E> c) {
    super(c);
        if (contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
    }

    /**
     * 
     */
    private static final long serialVersionUID = 7716772103636691126L;

    public boolean add(E e) {
        if (e == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.add(e);
    };

    @Override
    public boolean addAll(Collection<? extends E> c) {
        if (c.contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.addAll(c);
    }

    public void add(int index, E element) {
        if (element == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        super.add(index, element);
    };

    @Override
    public boolean addAll(int index, Collection<? extends E> c) {
        if (c.contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.addAll(index, c);
    }

    public E set(int index, E element) {
        if (element == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.set(index, element);
    };

    @Override
    public Object clone() {
        throw new Error(CLONING_IS_UNSUPPORTED);
    }

}

Besides I want to prevent code from using the clone() method because I just don't need it.

Is the implementation good or not?

Answer 1

• Throwing NullPointerException from methods like add, set are OK, cause the element to be added can't be null. But throwing NPE from addAll method is not cool. From Javadoc

  Throws: NullPointerException - if the specified collection is null

  that means if there exists a null value in the Collection throwing NPE doesn't make sense. You can throw IllegalArgumentException or a custom exception like FoundNullValueInCollectionException(I think this is better).

  In your defense you can point towards the Map.containsKey Javadoc, where it throws NPE if the specified key is null and this map does not permit null keys (optional).

---

• I want to prevent code from using the clone() method

  So you can throw a UnsupportedOperationException or CloneNotSupportedException(as JohnMark13 suggested).

Answer 2

In your case I think that you should consider the use of IllegalArgumentException it will save you a headache in the long run. When it comes to looking at code and trying to work out what broke and why NPEs are very well understood and very useful in debugging, but IAE describes your situation better.

The bottom line is that you have asked a question with no right or wrong answer and you will find very strong opinions form both camps - here is a good example of a short debate with many convincing opinions on StackOverflow

I'm not sure why the conversation around the clone method, if you want to extend ArrayList without clone you should throw the standard CloneNotSupportedException.

A similar discussion was had the other day, are you sure that you want to extend ArrayList at all, would you be better off having a private ArrayList member variable, delegating the methods that you care about to it and not exposing the others.