Самый читаемый способ написания простой условной проверки - PullRequest
6 голосов
/ 28 апреля 2009

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

Две возможности, о которых я мог подумать (это Java, но язык здесь не имеет значения):

Вариант 1:

   boolean c1 = passwordField.getPassword().length > 0;
   boolean c2 = !stationIDTextField.getText().trim().isEmpty();
   boolean c3 = !userNameTextField.getText().trim().isEmpty();

   if (c1 && c2 && c3) {
      okButton.setEnabled(true);
   }

Вариант 2:

   if (passwordField.getPassword().length > 0 &&
         !stationIDTextField.getText().trim().isEmpty() &&
         !userNameTextField.getText().trim().isEmpty() {
      okButton.setEnabled(true);
   }

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

Так что вы думаете? Любые другие варианты?

Ответы [ 7 ]

27 голосов
/ 28 апреля 2009
if (HasPassword() && HasStation() && HasUserName())
  okButton.setEnabled(true);


bool HasPassword() {
 return passwordField.getPassword().length > 0;
}

и т.д.

6 голосов
/ 28 апреля 2009

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

4 голосов
/ 28 апреля 2009

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

Так что мой код, вероятно, будет выглядеть так:

boolean enteredPassword = passwordField.getPassword().length > 0;
boolean stationIDIsEmpty = stationIDTextField.getText().trim().isEmpty();
boolean userNameIsEmpty = userNameTextField.getText().trim().isEmpty();

if (enteredPassword && !stationIDIsEmpty && !userNameIsEmpty) {
   okButton.setEnabled(true);
}
3 голосов
/ 28 апреля 2009

Я голосовал за ответ Криса Брандсмы.

Но я просто хотел упомянуть главную проблему, которая у меня возникла с вариантом 1: вы теряете преимущество &&. С первым вариантом, хотя я думаю, что он более читабелен, вы обрабатываете сравнения, когда они могут не потребоваться.

1 голос
/ 28 апреля 2009

Параметр 1 является основным для применения рефакторинга ' Замените темп на Запрос '. Причина в том, что кто-то может вставить код между переменной, инициализировать и проверять и изменять поведение кода. Или же проверка может быть сделана с устаревшими значениями ... было выполнено обновление текстовых полей между инициализацией и проверкой.

Так что моя попытка это будет

if (GetPasswordLength() > 0 
   && FieldHelper.IsNotEmpty(stationIDTextField) 
   && FieldHelper.IsNotEmpty(userNameTextField) 
{
   okButton.setEnabled(true);
}

FieldHelper - это класс с общедоступными статическими методами (также называемый классом Утилиты / Статическим классом в C #)

1 голос
/ 28 апреля 2009

Я предпочитаю следующее:

if (passwordField.getPassword().length > 0
    && ! stationIDTextField.getText().trim().isEmpty()
    && ! userNameTextField.getText().trim().isEmpty())
{
    okButton.setEnabled(true);
}

С этим стилем кодирования я выполняю две вещи:

  • Я легко вижу, что каждая дополнительная строка if является частью условия из-за && (или ||) в начале.
  • Я легко вижу, где заканчивается оператор if из-за {на следующей строке.
1 голос
/ 28 апреля 2009

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

То есть, с вашим вторым вариантом становится ясно, что ваше условное выражение примерно так: пусто, тогда ... "

...