1
\$\begingroup\$

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?

\$\endgroup\$
7
\$\begingroup\$
  • 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).


\$\endgroup\$
  • \$\begingroup\$ thanks. But any exception can be caught. Errors can't be caught. \$\endgroup\$ – Maksim Dmitriev Sep 2 '13 at 13:15
  • \$\begingroup\$ @RedPlanet and when did I say anything about Errors? \$\endgroup\$ – Anirban Nag 'tintinmj' Sep 2 '13 at 13:43
  • 4
    \$\begingroup\$ @RedPlanet of course Errors can be caught \$\endgroup\$ – bowmore Sep 2 '13 at 13:46
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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