ООП Дизайн Вопрос - Проверка свойств - PullRequest
3 голосов
/ 30 октября 2009

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

Пример:

public void setName(String newName){
   if (newName == null){
      throw new IllegalArgumentException("Name cannot be null");
   }
   name = newName;
}

...

//Only call to setName(String)
t.setName("Jim");

Ответы [ 8 ]

4 голосов
/ 31 октября 2009

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

2 голосов
/ 30 октября 2009

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

Но, конечно, это в значительной степени суждение, которое должно быть сделано в каждом конкретном случае. Вы можете видеть, что это (и я видел это) полностью вышло из-под контроля - до такой степени, что простая функция превращается в клубок операторов if, которые почти никогда не принимают значение true.

1 голос
/ 14 февраля 2013

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

Используя конкретный пример, который вы разместили, ИМХО вам следует выполнить проверку, но другим способом.

Ключ к архивированию валидации лежит в самом вопросе. Подумайте об этом: вы имеете дело с именами, а не со строками. Строка - это имя, когда оно не равно нулю. Мы также можем подумать о дополнительных характеристиках, которые делают строку именем: она не может быть пустой или содержать пробелы.

Предположим, вам нужно добавить эти правила проверки: если вы придерживаетесь своего подхода, вы в конечном итоге загромождаете свой сеттер, как сказал @SingleShot.

Кроме того, что бы вы сделали, если бы более чем один объект домена имел установщик setName? Даже если вы используете вспомогательные классы как @dave, код все равно будет дублироваться: вызовы вспомогательных экземпляров.

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

Помните, что вы имеете дело с именами, так почему бы не смоделировать концепцию имени? Вот ванильная, фиктивная реализация, чтобы показать идею:

public class Name

  public static Name From(String value) {
    if (string.IsNullOrEmpty(value)) throw new ...
    if (value.contains(' ')) throw new ...  

    return new Name(value);
  }

  private Name(string value) {
    this.value = value;
  }

  // other Name stuff goes here...  
}

Поскольку проверка происходит в момент создания, вы можете получить только действительные экземпляры Name. Нет способа создать экземпляр Name из «недопустимой» строки. Централизован не только проверочный код, но и исключения создаются в контексте, который имеет для них значение (создание экземпляра Name).

Вы можете прочитать о великолепных принципах дизайна в «Принципах проектирования позади Патагонии» Эрнана Уилкинсона (пример названия взят из него). Обязательно ознакомьтесь с ESUG 2010 Video и слайдами презентации

Наконец, я думаю, вам может показаться интересной статья Джима Шора "Fail Fast" .

1 голос
/ 30 октября 2009

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

1 голос
/ 30 октября 2009

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

0 голосов
/ 30 октября 2009

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

0 голосов
/ 30 октября 2009

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

0 голосов
/ 30 октября 2009

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

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

Раньше у меня были служебные классы, чтобы сводить код к минимуму. Так что вместо

if (name == null) { throw new ...

вы могли бы иметь

Util.assertNotNull(name)

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

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