Допустимо ли использовать исключения вместо подробных нулевых проверок? - PullRequest
12 голосов
/ 30 января 2012

Я недавно столкнулся с этой проблемой в проекте: есть цепочка вложенных объектов, например: класс A содержит переменную экземпляра класса B, которая в свою очередь имеет переменную экземпляра класса C, ..., пока у нас не появится узел в дереве класса Z.

     -----      -----      -----      -----               ----- 
     | A | ---> | B | ---> | C | ---> | D | ---> ... ---> | Z |
     -----      -----      -----      -----               -----  

Каждый класс предоставляет методы получения и установки для своих членов. Родительский экземпляр A создается анализатором XML, и для любого объекта в цепочке допустимо иметь значение null.

Теперь представьте, что в определенной точке приложения у нас есть ссылка на экземпляр A, и только если он содержит объект Z, мы должны вызвать метод для него. Используя регулярные проверки, мы получаем этот код:

    A parentObject;

    if(parentObject.getB() != null &&
        parentObject.getB().getC() != null &&
        parentObject.getB().getC().getD() != null &&
        parentObject.getB().getC().getD().getE() != null &&
        ...
        parentObject.getB().getC().getD().getE().get...getZ() != null){
            parentObject.getB().getC().getD().getE().get...getZ().doSomething();
    }

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

    try {
        parentObject.getB().getC().getD().getE().get...getZ().doSomething();
    } catch (NullPointerException e){}

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

Допустимо ли это делать для экономии времени разработки? Как можно изменить API, чтобы избежать этой проблемы?

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

(я использовал код Java, но тот же вопрос может применяться к свойствам C #)

Спасибо.

Ответы [ 9 ]

16 голосов
/ 30 января 2012

Плохой дизайн, если parentObject должен знать, что A содержит B, который содержит C, который содержит .... Таким образом, все связано со всем. Вы должны взглянуть на закон Деметры: http://en.wikipedia.org/wiki/Law_Of_Demeter

parentObject должен вызывать методы только для своей переменной экземпляра B. Таким образом, B должен предоставлять метод, который позволяет принять решение, например,

public class A {
  private B myB;
  //...
  public boolean isItValidToDoSomething(){
    if(myB!=null){
      return myB.isItValidToDoSomething();
    }else{
      return false;
    }
  }
}

В конце концов, на уровне Z, метод должен возвращать true.

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

4 голосов
/ 30 января 2012

Использовать здесь исключения - плохая практика.

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

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

Вопросы, которые вы можете задать:

  • Почему в любом случае вызывающему объекту необходимо выполнить функцию Some () с объектом Z?Почему бы не поместить doSomething () в класс A?Это может распространять doSomething () вниз по цепочке, если это необходимо, и если соответствующее поле не было пустым ....
  • Что означает нулевое значение означает , если оно существует в этой цепочке?Значение null будет подсказывать, какую бизнес-логику следует использовать для ее обработки .... которая может быть разной на каждом уровне.

В целом, я подозреваю, что правильный ответ - поставить doSomething ()на каждом уровне иерархии и реализации что-то вроде:

class A {
  ...
  public void doSomething() {
    B b=getB();
    if (b!=null) {
      b.doSomething();
    } else {
      // do default action in case of null B value
    }
  }
}

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

3 голосов
/ 30 января 2012

Ну, это зависит от того, что именно вы делаете в улове. В приведенном выше случае кажется, что вы хотите вызвать doSomething (), если он доступен, но если это не так, вам все равно. В этом случае я бы сказал, что отлавливать конкретное исключение, за которым вы работаете, так же приемлемо, как и многословную проверку, чтобы гарантировать, что вы не бросите ее с самого начала. Есть много «нулевых» методов и расширений, которые используют try-catch очень похоже на то, что вы предлагаете; Методы типа ValueOrDefault являются очень мощными обертками для именно того, что было сделано с помощью try-catch, именно по той причине, по которой использовался try-catch.

Try / catch по определению является оператором управления потоком программ. Следовательно, ожидается, что он будет использоваться для «управления обычным потоком программ»; Я думаю, что различие, которое вы пытаетесь сделать, состоит в том, что он не должен использоваться для управления «счастливым путем» нормального логического потока без ошибок. Даже тогда я могу не согласиться; В .NET Framework и сторонних библиотеках есть методы, которые либо возвращают желаемый результат, либо выдают исключение. «Исключение» не является «ошибкой», пока вы не можете продолжить из-за него; если есть что-то еще, что вы можете попробовать или какой-то случай по умолчанию, к которому может привести ситуация, это может считаться «нормальным» для получения исключения. Таким образом, catch-handle-continue - это вполне допустимое использование try-catch, и многие способы использования исключений в Framework ожидают, что вы справитесь с ними надежно.

Чего вы хотите избежать, так это использовать try / catch как «goto», генерируя исключения, которые на самом деле не являются исключениями, для того, чтобы «перейти» к оператору catch, когда какое-либо условие выполнено. Это определенно хак, и, следовательно, плохое программирование.

3 голосов
/ 30 января 2012

Лично мне нравится вообще избегать этой проблемы, используя параметр типа . Изменяя значение, возвращаемое этими методами / свойствами, чтобы оно было Option<T>, а не T, вызывающий абонент может выбрать, как он будет обрабатывать регистр без значения.

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

например. в C #:

class A {
    Option<B> B { get { return this.optB; } }
}

class B {
    Option<C> C { get { return this.optC; } }
}

// and so on

Если вызывающий хочет выбросить, он просто извлекает значение, не проверяя явно, есть ли оно:

A a = GetOne();
D d = a.Value.B.Value.C.Value.D.Value; // Value() will throw if there is no value

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

A a = GetOne();
D d = a.Convert(a => a.B) // gives the value or empty Option<B>
       .Convert(b => b.C) // gives value or empty Option<C>
       .Convert(c => c.D) // gives value or empty Option<D>
       .ValueOrDefault(new D("No value")); // get a default if anything was empty 

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

A a = GetOne();
D d = a.ValueOrDefault(defaultA)
     .B.ValueOrDefault(defaultB)
     .C.ValueOrDefault(defaultC)
     .D.ValueOrDefault(defaultD);

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

2 голосов
/ 03 февраля 2012

Использование исключений здесь не совсем подходит.Что, если один из получателей содержал нетривиальную логику и выдавал исключение NullPointerException?Ваш код проглотит это исключение без намерения.Что касается примечания, ваши примеры кода демонстрируют другое поведение, если parentObject равно null.

Кроме того, «телескоп» действительно не нужен:

public Z findZ(A a) {
    if (a == null) return null;
    B b = a.getB();
    if (b == null) return null;
    C c = b.getC();
    if (c == null) return null;
    D d = c.getD();
    if (d == null) return null;
    return d.getZ();
}
2 голосов
/ 31 января 2012

В Python они поощряют стиль «проще просить прощения, чем разрешения», который можно применить здесь, чтобы сказать, что лучше просто оптимистично попытаться добраться до Z без проверки безопасности, и позволить обработчику исключений исправить ошибку , Это проще для кодирования, и это более производительно, если вызов Z, не входящий в цепочку вызовов, менее вероятен, чем случай, в котором он будет.

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

Можете ли вы изменить язык тогда? C # со статической типизацией может быть не лучшим выбором для того, что вы здесь делаете. Возможно, использование Iron Python или другого языка, который немного менее удобен при наборе текста, позволит вам легче манипулировать DOM. Как только вы получите данные в стабильном состоянии, вы можете передать их C # для остальных.

2 голосов
/ 30 января 2012

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

Если вы должны сохранить эту очень глубокую иерархию, то вы можете использовать статические экземпляры каждого объекта, который определяет «пустое» состояние. Лучший пример, который я могу придумать, для чего это класс C # string, имеет статическое поле string.Empty. Тогда каждый вызов getB(), getC() ... getZ() будет возвращать либо реальное значение, либо состояние "пусто", что позволит вам связать вызовы методов.

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

0 голосов
/ 01 февраля 2012

Я согласен с другими ответами, что это не нужно делать, но если вам нужно, здесь есть вариант:

Вы можете создать метод перечислителя один раз, например:

    public IEnumerable<type> GetSubProperties(ClassA A)
    {
        yield return A;
        yield return A.B;
        yield return A.B.C;
        ...
        yield return A.B.C...Z;
    }

А затем используйте его как:

    var subProperties = GetSubProperties(parentObject);
    if(SubProperties.All(p => p != null))
    {
       SubProperties.Last().DoSomething();
    }

Перечислитель будет лениво оцениваться, что не приведет к исключениям.

0 голосов
/ 30 января 2012

Я думаю, вы могли бы предоставить статические isValid методы для каждого класса, например для класса A, который будет:

public class A {
...
  public static boolean isValid (A obj) {
    return obj != null && B.isValid(obj.getB());
  }
...
}

И так далее. Тогда у вас будет:

A parentObject;

if (A.isValid(parentObject)) {
  // whatever
}

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...