Нулевая цепочка проверок и перехват NullPointerException

Веб-сервис возвращает огромный XML, и мне нужно получить доступ к его глубоко вложенным полям. Например:

return wsObject.getFoo().getBar().getBaz().getInt()

Проблема в том, что getFoo(), getBar(), getBaz() могут все возвращать null .

Однако, если я проверю на null во всех случаях, код становится очень многословным и трудным для чтения. Более того, я могу пропустить проверки некоторых полей.

if (wsObject.getFoo() == null) return -1;
if (wsObject.getFoo().getBar() == null) return -1;
// maybe also do something with wsObject.getFoo().getBar()
if (wsObject.getFoo().getBar().getBaz() == null) return -1;
return wsObject.getFoo().getBar().getBaz().getInt();

Допустимо ли писать

try {
    return wsObject.getFoo().getBar().getBaz().getInt();
} catch (NullPointerException ignored) {
    return -1;
}

или это будет считаться антипаттерном?

102 голоса | спросил David Frank 22 J0000006Europe/Moscow 2016, 09:57:03

18 ответов


0

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

Я предлагаю вам использовать Optional вместо класса. Часто это лучший подход, когда вы хотите работать со значениями, которые присутствуют или отсутствуют.

Используя это, вы можете написать свой код следующим образом:

public Optional<Integer> m(Ws wsObject) {
    return Optional.ofNullable(wsObject.getFoo()) // Here you get Optional.empty() if the Foo is null
        .map(f -> f.getBar()) // Here you transform the optional or get empty if the Bar is null
        .map(b -> b.getBaz())
        .map(b -> b.getInt());
        // Add this if you want to return an -1 int instead of an empty optional if any is null
        // .orElse(-1);
        // Or this if you want to throw an exception instead
        // .orElseThrow(SomeApplicationException::new);
}

Почему необязательно?

Использование Optional вместо null для значений, которые могут отсутствовать, этот факт становится очень понятным и понятным читателям, а система типов гарантирует, что вы случайно не забудете об этом.

Вы также получите доступ к методам для более удобной работы с такими значениями, например map и orElse .


Допустимо ли отсутствие или ошибка?

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


Может быть, больше опций?

Если, с другой стороны, допустимые значения из промежуточных методов допустимы, может быть, вы также можете переключиться на Optional для них?

Тогда вы можете использовать их следующим образом:

public Optional<Integer> mo(Ws wsObject) {
    return wsObject.getFoo()
        .flatMap(f -> f.getBar())
        .flatMap(b -> b.getBaz())
        .flatMap(b -> b.getInt());        
}

Почему не необязательно?

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

ответил Lii 22 J0000006Europe/Moscow 2016, 11:00:19
0

Я предлагаю рассмотреть Objects.requireNonNull(T obj, String message) . Вы можете создавать цепочки с подробным сообщением для каждого исключения, например

requireNonNull(requireNonNull(requireNonNull(
    wsObject, "wsObject is null")
        .getFoo(), "getFoo() is null")
            .getBar(), "getBar() is null");

Я бы посоветовал вам не использовать специальные возвращаемые значения, такие как -1. Это не стиль Java. Ява разработала механизм исключений, чтобы избежать этого старомодного способа, который пришел из языка Си.

Бросать NullPointerException тоже не лучший вариант. Вы можете предоставить собственное исключение (сделав его проверенным , чтобы гарантировать, что оно будет обработано пользователем, или не проверено , чтобы обработать его более простым способом) или использовать конкретное исключение из Используемый вами парсер XML.

ответил Andrew Tobilko 22 J0000006Europe/Moscow 2016, 10:13:25
0

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

static <T> T get(Supplier<T> supplier, T defaultValue) {
    try {
        return supplier.get();
    } catch (NullPointerException e) {
        return defaultValue;
    }
}

Теперь вы можете просто сделать:

return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), -1);
ответил shmosel 23 J0000006Europe/Moscow 2016, 05:28:56
0

Как уже указывалось Томом в комментарии,

Следующее утверждение не подчиняется Закону Деметры ,

wsObject.getFoo().getBar().getBaz().getInt()

Вы хотите int, и вы можете получить его из Foo. Закон Деметры гласит: никогда не разговаривай с незнакомцами . Для вашего случая вы можете скрыть фактическую реализацию под капотом Foo и Bar.

Теперь вы можете создать метод в Foo для получения int из Baz. В конечном итоге Foo будет иметь Bar и Bar мы можем получить доступ к Int, не раскрывая ---- +: = 12 =: + ---- непосредственно к Baz. Таким образом, нулевые проверки, вероятно, разделены на разные классы, и только обязательные атрибуты будут разделены между классами.

ответил CoderCroc 22 J0000006Europe/Moscow 2016, 12:05:44
0

Мой ответ идет почти в той же строке, что и @janki, но я хотел бы немного изменить фрагмент кода, как показано ниже:

if (wsObject.getFoo() != null && wsObject.getFoo().getBar() != null && wsObject.getFoo().getBar().getBaz() != null) 
   return wsObject.getFoo().getBar().getBaz().getInt();
else
   return something or throw exception;

Вы также можете добавить нулевую проверку для wsObject, если есть вероятность того, что этот объект будет нулевым.

ответил Arka Ghosh 22 J0000006Europe/Moscow 2016, 10:27:01
0

Чтобы улучшить читабельность, вы можете использовать несколько переменных, например

Foo theFoo;
Bar theBar;
Baz theBaz;

theFoo = wsObject.getFoo();

if ( theFoo == null ) {
  // Exit.
}

theBar = theFoo.getBar();

if ( theBar == null ) {
  // Exit.
}

theBaz = theBar.getBaz();

if ( theBaz == null ) {
  // Exit.
}

return theBaz.getInt();
ответил JimmyB 22 J0000006Europe/Moscow 2016, 15:09:44
0

Не перехватывать NullPointerException. Вы не знаете, откуда это исходит (я знаю, что в вашем случае это маловероятно, но возможно что-то еще его бросило), и это медленно. Вы хотите получить доступ к указанному полю, и для этого каждое другое поле должно быть не нулевым. Это идеальная причина для проверки каждого поля. Я бы, наверное, проверил это в одном, а затем создал бы метод для удобства чтения. Как уже отмечали другие, возвращение -1 - это очень старая школа, но я не знаю, есть ли у вас причина для этого или нет (например, разговор с другой системой).

public int callService() {
    ...
    if(isValid(wsObject)){
        return wsObject.getFoo().getBar().getBaz().getInt();
    }
    return -1;
}


public boolean isValid(WsObject wsObject) {
    if(wsObject.getFoo() != null &&
        wsObject.getFoo().getBar() != null &&
        wsObject.getFoo().getBar().getBaz() != null) {
        return true;
    }
    return false;
}

Edit: Это спорно, если это disobeyes Закон Деметры, так как WsObject это, вероятно, только структура данных (проверка https: //stackoverflow.com/a/26021695/1528880 ).

ответил DerM 22 J0000006Europe/Moscow 2016, 10:58:49
0

Если вы не хотите проводить рефакторинг кода и можете использовать Java 8, можно использовать ссылки на методы.

Сначала простая демонстрация (извините за статические внутренние классы)

public class JavaApplication14 
{
    static class Baz
    {
        private final int _int;
        public Baz(int value){ _int = value; }
        public int getInt(){ return _int; }
    }
    static class Bar
    {
        private final Baz _baz;
        public Bar(Baz baz){ _baz = baz; }
        public Baz getBar(){ return _baz; }   
    }
    static class Foo
    {
        private final Bar _bar;
        public Foo(Bar bar){ _bar = bar; }
        public Bar getBar(){ return _bar; }   
    }
    static class WSObject
    {
        private final Foo _foo;
        public WSObject(Foo foo){ _foo = foo; }
        public Foo getFoo(){ return _foo; }
    }
    interface Getter<T, R>
    {
        R get(T value);
    }

    static class GetterResult<R>
    {
        public R result;
        public int lastIndex;
    }

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) 
    {
        WSObject wsObject = new WSObject(new Foo(new Bar(new Baz(241))));
        WSObject wsObjectNull = new WSObject(new Foo(null));

        GetterResult<Integer> intResult
                = getterChain(wsObject, WSObject::getFoo, Foo::getBar, Bar::getBar, Baz::getInt);

        GetterResult<Integer> intResult2
                = getterChain(wsObjectNull, WSObject::getFoo, Foo::getBar, Bar::getBar, Baz::getInt);


        System.out.println(intResult.result);
        System.out.println(intResult.lastIndex);

        System.out.println();
        System.out.println(intResult2.result);
        System.out.println(intResult2.lastIndex);

        // TODO code application logic here
    }

    public static <R, V1, V2, V3, V4> GetterResult<R>
            getterChain(V1 value, Getter<V1, V2> g1, Getter<V2, V3> g2, Getter<V3, V4> g3, Getter<V4, R> g4)
            {
                GetterResult result = new GetterResult<>();

                Object tmp = value;


                if (tmp == null)
                    return result;
                tmp = g1.get((V1)tmp);
                result.lastIndex++;


                if (tmp == null)
                    return result;
                tmp = g2.get((V2)tmp);
                result.lastIndex++;

                if (tmp == null)
                    return result;
                tmp = g3.get((V3)tmp);
                result.lastIndex++;

                if (tmp == null)
                    return result;
                tmp = g4.get((V4)tmp);
                result.lastIndex++;


                result.result = (R)tmp;

                return result;
            }
}

Выход

  

241
  4

     

нуль
  2

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

Метод getterChain - это простой шаблонный код, который может быть сгенерирован автоматически (или вручную при необходимости).
Я структурировал код так, чтобы повторяющийся блок был самоочевидным.


Это не идеальное решение, так как вам все равно нужно определить одну перегрузку getterChain на количество получателей.

Я бы вместо этого осуществил рефакторинг кода, но если нет, и вы часто обнаруживаете, что используете длинные цепочки геттеров, вы можете подумать о создании класса с перегрузками, которые принимают от 2 до, скажем, 10, геттеров.

ответил Margaret Bloom 22 J0000006Europe/Moscow 2016, 20:30:03
0

Как уже говорили другие, соблюдение закона Деметры определенно является частью решения. Другая часть, где это возможно, заключается в изменении этих сцепленных методов, чтобы они не могли возвращать null. Вы можете не возвращать null, вместо этого возвращая пустой код String, пустой Collection или какой-либо другой фиктивный объект, который означает или делает то, что вызывающий объект сделает с null

ответил Kevin Krumwiede 22 J0000006Europe/Moscow 2016, 21:01:30
0

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

Есть тысячи случаев, когда ваш код может работать неправильно: невозможно подключиться к базе данных, исключение ввода-вывода, ошибка сети ... Если вы работаете с ними один за другим (как проверка нуля здесь), это будет слишком много хлопот.

В коде:

wsObject.getFoo().getBar().getBaz().getInt();

Даже если вы знаете, какое поле пустое, вы не понимаете, что происходит не так. Может быть, Bar имеет значение null, но ожидается ли это? Или это ошибка данных? Подумайте о людях, которые читают ваш код

Как и в ответе xenteros, я бы предложил использовать настраиваемое непроверенное исключение . Например, в этой ситуации: Foo может быть нулевым (допустимые данные), но Bar и Baz никогда не должны быть нулевыми (недопустимые данные)

Код можно переписать:

void myFunction()
{
    try 
    {
        if (wsObject.getFoo() == null)
        {
          throw new FooNotExistException();
        }

        return wsObject.getFoo().getBar().getBaz().getInt();
    }
    catch (Exception ex)
    {
        log.error(ex.Message, ex); // Write log to track whatever exception happening
        throw new OperationFailedException("The requested operation failed")
    }
}


void Main()
{
    try
    {
        myFunction();
    }
    catch(FooNotExistException)
    {
        // Show error: "Your foo does not exist, please check"
    }
    catch(OperationFailedException)
    {
        // Show error: "Operation failed, please contact our support"
    }
}
ответил Hoàng Long 23 J0000006Europe/Moscow 2016, 05:08:14
0

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

Вам придется перехватывать исключение везде, где вы хотите вызвать метод (или он будет распространяться вверх по стеку). Тем не менее, если в вашем случае вы можете продолжать работать с этим результатом со значением -1, и вы уверены, что он не будет распространяться, потому что вы не используете ни одну из «частей», которые могут быть нулевыми, то мне кажется правильным лови это

Edit:

Я согласен с более поздним ответом от @xenteros, лучше будет запустить собственное исключение вместо возврата -1 Вы можете назвать его InvalidXMLException, например.

ответил SCouto 22 J0000006Europe/Moscow 2016, 10:01:44
0

Следите за этим сообщением со вчерашнего дня.

Я комментирую /голосую за комментарии, в которых говорится, что ловить NPE - это плохо. Вот почему я это делал.

package com.todelete;

public class Test {
    public static void main(String[] args) {
        Address address = new Address();
        address.setSomeCrap(null);
        Person person = new Person();
        person.setAddress(address);
        long startTime = System.currentTimeMillis();
        for (int i = 0; i < 1000000; i++) {
            try {
                System.out.println(person.getAddress().getSomeCrap().getCrap());
            } catch (NullPointerException npe) {

            }
        }
        long endTime = System.currentTimeMillis();
        System.out.println((endTime - startTime) / 1000F);
        long startTime1 = System.currentTimeMillis();
        for (int i = 0; i < 1000000; i++) {
            if (person != null) {
                Address address1 = person.getAddress();
                if (address1 != null) {
                    SomeCrap someCrap2 = address1.getSomeCrap();
                    if (someCrap2 != null) {
                        System.out.println(someCrap2.getCrap());
                    }
                }
            }
        }
        long endTime1 = System.currentTimeMillis();
        System.out.println((endTime1 - startTime1) / 1000F);
    }
}

  public class Person {
    private Address address;

    public Address getAddress() {
        return address;
    }

    public void setAddress(Address address) {
        this.address = address;
    }
}

package com.todelete;

public class Address {
    private SomeCrap someCrap;

    public SomeCrap getSomeCrap() {
        return someCrap;
    }

    public void setSomeCrap(SomeCrap someCrap) {
        this.someCrap = someCrap;
    }
}

package com.todelete;

public class SomeCrap {
    private String crap;

    public String getCrap() {
        return crap;
    }

    public void setCrap(String crap) {
        this.crap = crap;
    }
}

Выход

3,216

0,002

Я вижу здесь явного победителя. Наличие проверок намного дешевле, чем ловить исключение. Я видел этот способ Java-8. Учитывая, что 70% текущих приложений все еще работают на Java-7, я добавляю этот ответ.

Итог . Для любых критически важных приложений обработка NPE обходится дорого.

ответил NewUser 23 J0000006Europe/Moscow 2016, 15:08:39
0

Если проблема заключается в эффективности, следует рассмотреть вариант «поймать». Если «catch» нельзя использовать, потому что он будет распространяться (как упомянуто в «SCouto»), тогда используйте локальные переменные, чтобы избежать многократных вызовов методов getFoo(), getBar() и getBaz().

ответил JEP 22 J0000006Europe/Moscow 2016, 10:09:24
0

Стоит задуматься о создании собственного исключения. Давайте назовем это MyOperationFailedException. Вы можете бросить его вместо того, чтобы вернуть значение. Результат будет таким же - вы выйдете из функции, но не вернете жестко закодированное значение -1, которое является антипаттерном Java. В Java мы используем исключения.

try {
    return wsObject.getFoo().getBar().getBaz().getInt();
} catch (NullPointerException ignored) {
    throw new MyOperationFailedException();
}

EDIT:

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

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

Если это приемлемо, вас не волнует, где появился этот ноль. Если вы это сделаете, вам определенно не следует связывать эти запросы.

ответил xenteros 22 J0000006Europe/Moscow 2016, 10:07:21
0

Метод, который вы используете, является длинным, но очень читабельным. Если бы я был новым разработчиком, пришедшим на вашу кодовую базу, я бы понял, что вы делаете довольно быстро. Большинство других ответов (включая перехват исключения), по-моему, не делают вещи более читабельными, а некоторые, по моему мнению, делают их менее читаемыми.

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

private int getFooBarBazInt() {
    if (wsObject.getFoo() == null) return -1;
    if (wsObject.getFoo().getBar() == null) return -1;
    if (wsObject.getFoo().getBar().getBaz() == null) return -1;
    return wsObject.getFoo().getBar().getBaz().getInt();
}

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

Когда вы общаетесь с удаленным веб-сервисом, очень типично иметь «удаленный домен» и «домен приложения» и переключаться между ними. Удаленный домен часто ограничен веб-протоколом (например, вы не можете отправлять вспомогательные методы назад и вперед в чистом сервисе RESTful, а глубоко вложенные объектные модели являются общими, чтобы избежать множественных вызовов API), и поэтому не идеальны для прямого использования в ваш клиент.

Например:

public static class MyFoo {

    private int barBazInt;

    public MyFoo(Foo foo) {
        this.barBazInt = parseBarBazInt();
    }

    public int getBarBazInt() {
        return barBazInt;
    }

    private int parseFooBarBazInt(Foo foo) {
        if (foo() == null) return -1;
        if (foo().getBar() == null) return -1;
        if (foo().getBar().getBaz() == null) return -1;
        return foo().getBar().getBaz().getInt();
    }

}
ответил Pace 23 J0000006Europe/Moscow 2016, 17:47:28
0
return wsObject.getFooBarBazInt();

применяя закон Деметры,

class WsObject
{
    FooObject foo;
    ..
    Integer getFooBarBazInt()
    {
        if(foo != null) return foo.getBarBazInt();
        else return null;
    }
}

class FooObject
{
    BarObject bar;
    ..
    Integer getBarBazInt()
    {
        if(bar != null) return bar.getBazInt();
        else return null;
    }
}

class BarObject
{
    BazObject baz;
    ..
    Integer getBazInt()
    {
        if(baz != null) return baz.getInt();
        else return null;
    }
}

class BazObject
{
    Integer myInt;
    ..
    Integer getInt()
    {
        return myInt;
    }
}
ответил Khaled.K 29 J0000006Europe/Moscow 2016, 10:52:47
0

Давать ответ, который кажется отличным от всех остальных.

  

Я рекомендую вам проверить наличие NULL в if s.

Причина:

  

Мы не должны оставлять ни единого шанса для сбоя нашей программы.   NullPointer генерируется системой. Поведение системы   сгенерированные исключения не могут быть предсказаны . Вы не должны оставлять свои   программа в руках системы, когда у вас уже есть способ обработки   это по своему усмотрению. И установите механизм обработки исключений для дополнительной безопасности. !!

Чтобы облегчить чтение кода, попробуйте выполнить следующие условия:

if (wsObject.getFoo() == null || wsObject.getFoo().getBar() == null || wsObject.getFoo().getBar().getBaz() == null) 
   return -1;
else 
   return wsObject.getFoo().getBar().getBaz().getInt();

РЕДАКТИРОВАТЬ:

  

Здесь вам нужно сохранить эти значения wsObject.getFoo(),   wsObject.getFoo().getBar(), wsObject.getFoo().getBar().getBaz() в   некоторые переменные. Я не делаю это, потому что я не знаю, возвращение   типы этих функций.

Любые предложения будут оценены .. !!

ответил Janki Gadhiya 22 J0000006Europe/Moscow 2016, 10:18:29
0

Я написал класс с именем Snag, который позволяет вам определить путь для навигации по дереву объектов. Вот пример его использования:

Snag<Car, String> ENGINE_NAME = Snag.createForAndReturn(Car.class, String.class).toGet("engine.name").andReturnNullIfMissing();

Это означает, что экземпляр ENGINE_NAME будет эффективно вызывать Car?.getEngine()?.getName() переданного ему экземпляра и верните null, если какая-либо ссылка вернула null:

final String name =  ENGINE_NAME.get(firstCar);

Это не опубликовано на Maven, но если кто-то найдет это полезным, это здесь (без гарантии, конечно!)

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

ответил Rich 23 J0000006Europe/Moscow 2016, 16:42:50

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132