Каковы плюсы и минусы для различных методов проверки коллекции на нулевое значение перед добавлением в набор?

Question

Это дополнительный вопрос к " Существует ли базовая реализация Java Set, которая не допускает нулевые значения? ". (спасибо всем, кто помог с ответом)

Кажется, что способ сделать это, так как Sun не предоставил чего-то простого, такого как этот OOTB, это использовать оболочку / прокси. Это кажется почти прямым. Что мне интересно, каковы плюсы / минусы следующих двух подходов добавления коллекции, или есть другой лучший подход?

Подход № 1

public boolean addAll( Collection<? extends E> c) {
    if ( null == c ) {
        throw new NullPointerException( "c cannot be null" );
    }

    /*
     * This seems like a terrible abuse of exceptions when
     * all I want to do is check the set for a null.
     * 
     * However, simply running through each element of the
     * Collection may provide much worse performance for large
     * collections. And according to the Collection API, the
     * contains method may throw NullPointerExceptions if the
     * collection implementation does not allow null elements.
     */
    boolean collectionContainsNull = false;

    try {
        collectionContainsNull = c.contains(null);
    } catch (NullPointerException safeToIgnore) {
        /* Safe to ignore since we do not want nulls */
    }

    if (collectionContainsNull) {
        throw new NullPointerException("c cannot contain null elements");
    }

    this.wrapperSet.addAll(c);
}

Подход № 2

public boolean addAll( Collection<? extends E> c) {
    if ( null == c ) {
        throw new NullPointerException( "c cannot be null" );
    }

     E[] a = ( E[] )c.toArray();

     /*
     * We have to iterate through the entire collection to check for
     * a null. This won't take advantage of any optimizations that
     * c.contains may be using.
     *
     * We don't call add(e) here because if we hit a null midway through
     * we would violate the condition that the set remains unchanged
     * on error.
     */
     for ( E e : a ) {
         if (null == e) {
              throw new NullPointerException("c cannot contain nulls");
         }
     }

     this.wrapperSet.addAll(a);
}

Заранее спасибо!

Answer 1

Второй подход лучше. Никогда не скрывайте исключения - вы полагаетесь на предположение, что c.contains (null) генерирует исключение NullPointerException только в том случае, если в коллекции есть ноль. Однако если исключение NullPointeException вызывается из-за проблемы с методом equals, в вашем коде будет ошибка - и она будет скрыта.

Edit:

Из JavaDoc для содержит выбрасывается исключение NullPointerException - если указанный элемент имеет значение null и эта коллекция не допускает нулевые элементы (необязательно).

Учитывая, что это необязательный метод, вы можете получить исключение UnsupportedOperationException вместо NullPointerExcepion (в дополнение к сокрытию ошибки в equals).

Answer 2

Сделать копию коллекции аргументов в массиве безопаснее. Аргумент коллекции может изменяться одновременно (возможно, это одновременная коллекция) (или может быть написан злонамеренно).

Кроме того, перехват исключений во время выполнения не является хорошим способом сделать что-либо.

Возможно, вы захотите использовать Object[] вместо E[] и перенести небезопасный бросок на более поздний. Вам также понадобится Arrays.asList(a) или аналогичный.

Answer 3

Какой смысл сначала преобразовывать в массив, а затем выполнять итерацию по массиву, а не просто повторять коллекцию? Я бы сделал второй без постороннего преобразования.

Или, может быть, сделать добавление во временный набор:

public boolean addAll( Collection<? extends E> c) {
    if ( null == c ) {
        throw new NullPointerException( "c cannot be null" );
    }

    Set<E> tempSet = new HashSet<E>();

     /*
     * We have to iterate through the entire collection to check for
     * a null. This won't take advantage of any optimizations that
     * c.contains may be using.
     *

     */
     for ( E e : c) {
         if (null == e) {
              throw new NullPointerException("c cannot contain nulls");
         }
         tempSet.add(e);
     }

     this.wrapperSet = tempSet;
}

Answer 4

Так что, похоже, оба метода плохие. Я действительно публикую этот ответ только для того, чтобы объединить информацию в один пост.

TofuBeer указал на упущенный недостаток в логике для метода 1, где есть другие исключения, которые могут быть выброшены, которые не будут перехвачены. В общем, всегда плохо пытаться отловить исключения для исключительных условий.

Пол указал, что то, что я считал безопасным составом, на самом деле не так. Я ожидал, что универсальный набор будет применен к выходному приведению, однако он вернет Object []. Как он указывает, я мог бы использовать временный набор для хранения данных, пока я ищу ноль.

Кроме того, как подтвердил Том, вполне возможно, что аргумент коллекции может изменяться одновременно, так что защитная копия в новый объект является хорошей идеей).

Итак, я предполагаю, что желаемый метод:

public boolean addAll( Collection<? extends E> c ) {
    if ( null == c ) {
        throw new NullPointerException( "c cannot be null" );
    }

     // Create a defensive copy to prevent concurrent modifications of c
     Set<E> tmpSet = new HashSet<E>( c );

     /*
      * We have to iterate through the entire collection to check for
      * a null. This won't take advantage of any optimizations that
      * c.contains may be using.
      */
     for ( E e : tmpSet ) {
         if ( null == e ) {
              throw new NullPointerException( "c cannot contain nulls" );
         }
     }

     return this.wrapperSet.addAll( tmpSet );
}

Answer 5

Вы также можете добавить короткое замыкание, чтобы не проверять нулевые значения, если добавляемая коллекция подклассов вашего родительского класса NonNullCollection.