Проверьте нулевой объект и нулевое значение, содержащиеся в объекте в Java 8-way

как переписать эту функцию на большее количество Java 8 с опциями? Или я должен просто оставить его как есть?

public void setMemory(ArrayList<Integer> memory) {
    if (memory == null)
        throw new IllegalArgumentException("ERROR: memory object can't be null.");
    if (memory.contains(null))
        throw new IllegalArgumentException("ERROR: memory object can't contain null value.");

    this.memory = memory;
}

Ответ 1

Обычно я избегаю Optional для таких случаев, когда он имеет тенденцию скрывать, что происходит.

Но сначала я хотел бы упомянуть, что исходный код позволяет вызывающему абоненту сохранить ссылку на то, что теперь является внутренним полем memory содержащего класса. Возможно, вы доверяете своим абонентам не быть злонамеренными, но вызывающий может случайно использовать список, переданный в качестве аргумента. Если это произойдет, несмотря на тщательную проверку аргументов, список memory может в конечном итоге содержать нулевые значения. Или это может неожиданно измениться, что приведет к другим неполадкам.

Решение состоит в том, чтобы создать защитную копию списка аргументов. Прямой способ сделать это выглядит следующим образом:

public void setMemory(ArrayList<Integer> memory) {
    if (memory == null)
        throw new IllegalArgumentException("memory is null");

    List<Integer> temp = new ArrayList<>(memory);

    if (temp.contains(null))
        throw new IllegalArgumentException("memory contains null");

    this.memory = temp;
}

Обратите внимание, что копия создается и сохраняется в локальной переменной temp перед проверкой. Очевидно, что вы не хотите хранить в поле, прежде чем список будет проверен на предмет наличия нулей. Но проверка на наличие нулей должна выполняться в копии, а не в списке аргументов, в противном случае вызывающий может изменить список после проверки, но перед копией. (Да, это параноидально.)

Если вам не важно точное сообщение об исключении, это можно сократить следующим образом:

public void setMemory(ArrayList<Integer> memory) {
    List<Integer> temp;
    if (memory == null || ((temp = new ArrayList<>(memory)).contains(null)))
        throw new IllegalArgumentException("memory is or contains null");
    this.memory = temp;
}

Теперь это можно переписать для использования Optional:

public void setMemory(ArrayList<Integer> memory) {
    this.memory = Optional.ofNullable(memory)
                          .map(ArrayList::new)
                          .filter(list -> ! list.contains(null))
                          .orElseThrow(() -> new IllegalArgumentException("memory is or contains null"));
}

По сравнению с обычными злоупотреблениями:-) of Optional Я часто вижу это, это не так уж плохо. Цепочка здесь служит для того, чтобы избежать создания локальной переменной, которая немного выигрывает. Логика довольно проста, особенно если на переднем мозге есть Optional. Тем не менее, я был бы немного обеспокоен пересмотром этого кода, скажем, через месяц. Вероятно, вам придется немного прищуриться, прежде чем убедить себя, что делает то, что вы намеревались сделать.

Наконец, пара общих комментариев стиля.

Обычно предпочтение (по крайней мере, в JDK) заключается в использовании NullPointerException для этих случаев. Я придерживался IllegalArgumentException для этих примеров, потому что это то, что использует OP.
Я бы рекомендовал использовать List<Integer> вместо ArrayList<Integer> для типа аргумента и, возможно, для типа поля. Это позволит использовать немодифицируемые списки в ситуациях, где это необходимо (например, с использованием JDK 9 List.of).

Ответ 2

У вас есть шаблон condition -> throw an exception, который можно перенести в метод:

private void checkOrElseThrow(boolean condition, Supplier<? extends RuntimeException> exceptionSupplier) {
    if (condition) {
        throw exceptionSupplier.get();
    }
}

public void setMemory(List<Integer> memory) {

    checkOrElseThrow(memory == null, () -> new IllegalArgumentException("message #1"));
    checkOrElseThrow(memory.contains(null), () -> new IllegalArgumentException("message #2"));

    this.memory = memory;
}

Если тип исключения не будет изменен, разумно передать только сообщение об исключении (спасибо @tobias_k за указание его):

private void checkOrElseThrow(boolean condition, String exceptionMessage) {
    if (condition) {
        throw new IllegalArgumentException(exceptionMessage);
    }
}

public void setMemory(List<Integer> memory) {

    checkOrElseThrow(memory == null, "message #1");
    checkOrElseThrow(memory.contains(null), "message #2");

    this.memory = memory;
}

Ответ 3

Если вы хотите придерживаться IllegalArgumentException и у вас есть guava на пути к классу, вы можете использовать это:

Preconditions.checkArgument(memory != null, 
            "ERROR: memory object can't be null.");
Preconditions.checkArgument(!memory.contains(null), 
            "ERROR: memory object can't contain null value.");

Вы не можете использовать Optional здесь, так как вам нужны разные сообщения об ошибках для разных условий.

Если у вас есть одно сообщение об ошибке, с другой стороны, вы можете сделать:

this.memory = Optional.ofNullable(memory)
            .filter(x -> !x.contains(null))
            .orElseThrow(() -> new IllegalArgumentException(
                         "memory object is null or contains null values"));

Ответ 4

В первом случае я бы использовал: Objects.requireNonNull().

Я не думаю, что Optional - это способ пойти здесь, поскольку null является незаконным значением.

Ответ 5

Во-первых, может быть хорошей идеей использовать более общий тип списка в качестве входного параметра, поэтому измените свою реализацию на:

public void setMemory(List<Integer> memory) {
    //stuff
}

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

Если этот "список памяти" исходит из некоторого кода, и вы можете использовать guava, то, возможно, используйте список guavas неизменяемый. Этот список генерирует исключение, когда кто-то пытается добавить "нуль" в ваш список.

ImmutableList.of( //your Integers)

Если вы не можете использовать guava, но хотите использовать этот подход, вы всегда можете написать свою собственную реализацию списка, которая делает эту нулевую проверку для вас.

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

Ответ 6

Не используйте дополнительные, они не принесут вам пользу здесь.

Вместо этого используйте более подходящий тип вместо ArrayList. Хранение целых чисел в коллекции несет (не) затраты на упаковку и не имеет смысла, когда пустые значения не разрешены.

Несколько возможных библиотек коллекций, которые могут лучше удовлетворить ваши потребности:

Коллекции HPPC (мой любимый, но API несовместим с платформой Java Collection)
Koloboke
Fastutil

Все эти библиотеки предоставляют специализированные реализации списков, карт и других контейнеров для примитивов. Эти реализации, как правило, значительно быстрее, чем все, что включает ArrayList<Integer> (если только все целые числа в вашем ArrayList не достаточно малы, чтобы поместиться в глобальный кэш экземпляра Integer).

Приятным побочным эффектом является использование специализированных списков примитивных целых чисел, которые не позволят вызывающей стороне хранить значения по умолчанию.

Ответ 7

Один вкладыш с опциями:

public void setMemory(ArrayList<Integer> memory) {
    this.memory = Optional.ofNullable(memory).map((a) -> Optional.ofNullable(a.contains(null) ? null : a).orElseThrow(() -> new IllegalArgumentException("ERROR: memory object can't contain null value."))).orElseThrow(() -> new IllegalArgumentException("ERROR: memory object can't be null."));
}

Ответ 8

Извините за добавление еще одного ответа, но на основе чтения комментариев к вопросу может быть еще лучший способ изменить подпись метода: замените ArrayList<Integer> на IntStream:

public void setMemory(@NonNull IntStream input) {
    Objects.requireNonNull(input);

    this.memory = ...; // collect the stream into the storage
}

Примитивные потоки не несут стоимость (un) бокса.

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

Ответ 9

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

.map(e -> false) отображает элемент списка (целое в этом случае) на boolean, который требуется для filter().

this.memory = Optional.ofNullable(memory)
            .orElseThrow(() -> new IllegalArgumentException("ERROR: memory object can't be null."))
            .stream()
            .filter(element -> 
                    Optional.ofNullable(element)
                    .map(e -> true)
                    .orElseThrow(
                            () -> new IllegalArgumentException("ERROR: memory object can't contain null value.")))
            .collect(Collectors.toList());