Проверка, возвращает ли метод значение false: присваивать результат временной переменной или помещать вызов метода непосредственно в условное выражение?

9

Является ли хорошей практикой вызывать метод, который возвращает значения true или false в операторе if?

Что-то вроде этого:

private void VerifyAccount()
{
    if (!ValidateCredentials(txtUser.Text, txtPassword.Text))
    {
        MessageBox.Show("Invalid user name or password");
    }
}

private bool ValidateCredentials(string userName, string password)
{
    string existingPassword = GetUserPassword(userName);
    if (existingPassword == null)
        return false;

    var hasher = new Hasher { SaltSize = 16 };
    bool passwordsMatch = hasher.CompareStringToHash(password, existingPassword);

    return passwordsMatch;
}

или лучше сохранить их в переменной, а затем сравнить их, используя значения if else, как это

bool validate = ValidateCredentials(txtUser.Text, txtPassword.Text);
if(validate == false){
    //Do something
}

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

KyelJmD
источник
3
Если вы используете временную переменную, напишите if (!validate)вместо if (validate == false).
Филипп
12
Я бы назвал эту функцию как «CredentialsAreValid ()», чтобы вы знали, что она должна возвращать bool, но в противном случае да, это хорошая практика
Захари К
3
IsValidCredentials, хотя грамматически неудобно, является общим форматом для указания логического возвращаемого значения.
zzzzBov
@Philip В чем разница между if (! Validate) и if (validate) ??
KyelJmD
!является оператором «НЕ», он отрицает любое логическое выражение. Так же if (!validate)как и противоположность if (validate). Оператор if будет введен, если validateне верно.
Филипп

Ответы:

26

Как со всеми этими вещами это зависит.

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

Код не будет заметно менее эффективным.

ChrisF
источник
1
это тип для чтения? как тот, что я сделал выше
KyelJmD
@KyelJmD: Это зависит от культуры, в которой вы программируете. В местах, где я программировал, !ValidateCredentialsболее читабелен, потому что он явно говорит о том, что он делает в коде. Это очень ясно. Если вызываемая вами функция не имеет «самодокументируемого» имени, то лучше использовать переменную. В нынешнем виде я бы порекомендовал опустить переменную и использовать оставшийся самодокументированный код.
Джоэл Этертон
ValidateCredentials не читается в первую очередь - как имя говорит вам, что означает результат? Гораздо лучше либо CredentialsAreValid или CredentialsAreInvalid.
gnasher729
9

Зачем использовать дополнительную переменную? Я предпочитаю использовать первый подход, он более читабелен и прост.

Диего Пачеко
источник
4

Является ли хорошей практикой вызывать метод, который возвращает значения true или false в операторе if?

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

или лучше сохранить их в переменной, а затем сравнить их, используя значения if else, как это

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

dietbuddha
источник
2

Давай посмотрим ...

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

Но тогда, потому что вы СУХОЙ , если вы позже звонили ValidateCredentialsи печатали, ValidateCredentials(txtUser.Text, txtPassword.Text)вы знаете, что вы должны были создать дополнительную переменную.

kowsheek
источник
2

Да, обычно хорошо использовать такие методы, как будто условия. Это полезно, хотя, если имя метода указывает, что метод возвращает bool; например CanValidateCredentials. В языках стиля C этот метод часто принимает форму префиксов Is и Can, а в Ruby - '?' суффикс.

Кристиан Хорсдал
источник
1
Хорошие замечания по поводу имен методов, но я бы никогда не начал имя метода с «если». Тогда код гласит «если если». "Может" это нормально if (cat.canOpenCanOfCatFood()).
Кевин Клайн
Спасибо, @Kevin. Я имел в виду «есть», а не «если». Я отредактировал ответ
Кристиан Хорсдал
1

Есть еще одна проблема, которая еще не была отмечена: оценка короткого замыкания . В чем разница между этими двумя фрагментами кода?

Пример № 1:

if(foo() && bar() && baz())
{
    quz();
}

Пример № 2:

bool isFoo = foo();
bool isBar = bar();
bool isBaz = baz();
if(isFoo && isBar && isBaz)
{
    quz();
}

Эти два фрагмента кода, кажется, делают то же самое, но если ваш язык поддерживает оценку короткого замыкания, эти два фрагмента различны. Оценка короткого замыкания означает, что код оценит минимум, необходимый для прохождения или отказа условия. Если в примере № 1, если foo () возвращает false, bar () и baz () даже не оцениваются. Это полезно, если baz () является долгосрочным вызовом функции, потому что вы можете пропустить его, если либо foo () возвращает false, либо foo () возвращает true, а bar () возвращает false.

Это не так в примере № 2. foo (), bar () и baz () всегда оцениваются. Если вы ожидаете, что bar () и baz () будут демонстрировать побочные эффекты, у вас будут проблемы с примером №1. Пример №1 выше фактически эквивалентен этому:

Пример № 3:

if(foo())
{
    if(bar())
    {
        if(baz())
        {
            quz();
        }
    }
}

Помните об этих различиях в вашем коде при выборе между примерами № 1 и № 2.

user2023861
источник
1

Немного расширив тему читабельности ...

Я могу придумать две веские причины для сохранения результата в переменной:

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

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

Например, это:

bool foo_is_ok = is_ok(foo);
if (foo_is_ok) ...

не помогает читабельность, но это:

bool done_processing = feof(file1) && feof(file2);
if (done_processing) ...

возможно, так как это не сразу очевидно, это feof(file1) && feof(file2)означает, что мы закончили обработку.

passwordsMatchПеременной в вопросе, вероятно, лучший пример , чем мой.

Переменные одноразового использования полезны, если они дают значимое имя некоторому значению. (Это, конечно, на благо читателя.)

Кит Томпсон
источник
Я думаю, что я бы предпочел оставить комментарий, а не вводить done_processingпеременную. В конце концов, имя done_processingвыполняет функцию комментария. И реальный комментарий позволяет мне сказать одно или два слова о том, почему это условие сигнализирует о том, что мы закончили с обработкой. Не то чтобы я был великим комментатором, хотя, вероятно, я бы не делал ни в реальном коде ...
cmaster - восстановить монику
@cmaster: Одна из проблем с комментариями состоит в том, что они могут очень легко не синхронизироваться с кодом. Переменная named done_processingявляется более надежным способом выражения намерения.
Кит Томпсон
+1 за точку № 2 - иногда хорошо названная временная переменная действительно проясняет ситуацию.
user949300