Comprobando si un método devuelve falso: ¿asignar el resultado a la variable temporal o poner la invocación del método directamente en condicional?

9

¿Es una buena práctica llamar a un método que devuelve valores verdaderos o falsos en una declaración if?

Algo como esto:

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;
}

o es mejor almacenarlos en una variable y luego compararlos con valores if if this como este

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

No solo me refiero a .NET, me refiero a la pregunta en todos los lenguajes de programación, simplemente sucede que utilicé .NET como ejemplo

KyelJmD
fuente
3
Si utiliza una variable temporal, escriba en if (!validate)lugar de if (validate == false).
Philip
12
Llamaría a la función algo así como "CredentialsAreValid ()" para que sepa que debería devolver un bool, pero de lo contrario sí, es una buena práctica
Zachary K
3
IsValidCredentials, aunque gramaticalmente incómodo, es un formato común para indicar un valor de retorno booleano.
zzzzBov
@Philip, ¿cuál es la diferencia entre if (! Validate) y this if (validate) ??
KyelJmD
!es el operador "NO", niega cualquier expresión booleana. Así if (!validate)es lo contrario de if (validate). La declaración if se ingresará si validateno es verdadera.
Philip

Respuestas:

26

Como con todas estas cosas, depende.

Si no va a utilizar el resultado de su llamada, ValidateCredentialsentonces no es necesario (excepto para fines de depuración) almacenar el resultado en una variable local. Sin embargo, si hace que el código sea más legible (y, por lo tanto, más fácil de mantener) para que una variable vaya con eso.

El código no será mensurablemente menos eficiente.

ChrisF
fuente
1
¿Es ese tipo de legible? como el que hice arriba
KyelJmD
@KyelJmD: Eso depende de la cultura en la que esté programando. En los lugares en los que he programado, !ValidateCredentialses más legible porque dice explícitamente lo que está haciendo en el código. Está muy claro. Si la función que está llamando no tiene un nombre de "autodocumentación", entonces podría ser mejor ir con la variable. Tal como están las cosas, recomendaría omitir la variable y permanecer con el código de autodocumentación que tiene.
Joel Etherton
ValidateCredentials no es legible en primer lugar, ¿cómo le dice el nombre lo que significa el resultado? Mucho mejor ya sea CredentialsAreValid o CredentialsAreInvalid.
gnasher729
9

¿Por qué usar una variable adicional? Prefiero usar el primer enfoque, es más legible y simple.

Diego Pacheco
fuente
4

¿Es una buena práctica llamar a un método que devuelve valores verdaderos o falsos en una declaración if?

Sí, si el condicional no es lo suficientemente simple como para alinearlo y hacer que sea legible.

o es mejor almacenarlos en una variable y luego compararlos con valores if if this como este

Solo debe hacer esto si está utilizando el valor en varios lugares o si lo necesita para que el código sea más legible. De lo contrario, la asignación a una variable es innecesaria. El código innecesario es, en el mejor de los casos, un desperdicio y, en el peor, una fuente de un defecto.

dietbuddha
fuente
2

Vamos a ver...

Porque todo se trata de KISS , no hay necesidad de crear una variable adicional cuando puede prescindir de ella. Además, no hay necesidad de escribir más ... cuando no hay necesidad.

Pero luego, porque SECO , si luego estaba llamando ValidateCredentialsy se encuentra escribiendo ValidateCredentials(txtUser.Text, txtPassword.Text), sabe que debería haber creado una variable adicional.

kowsheek
fuente
2

Sí, generalmente está bien usar métodos como si fueran condiciones. Sin embargo, es útil si el nombre del método indica que el método devuelve un valor bool; por ejemplo CanValidateCredentials. En los lenguajes de estilo C, este método a menudo toma la forma de los prefijos Is y Can, y en Ruby con el '?' sufijo.

Christian Horsdal
fuente
1
Buen punto sobre los nombres de métodos, pero nunca comenzaría un nombre de método con "if". Luego, el código dice "if if". "Can" está bien: if (cat.canOpenCanOfCatFood()).
Kevin Cline
Gracias @Kevin. Quise decir 'es' no 'si'. Edité la respuesta
Christian Horsdal
1

Hay otra preocupación que aún no se ha señalado: la evaluación de cortocircuito . ¿Cuál es la diferencia entre estos dos fragmentos de código?

Ej # 1:

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

Ej # 2:

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

Parece que estos dos fragmentos de código hacen lo mismo, pero si su idioma admite la evaluación de cortocircuito, estos dos fragmentos son diferentes. La evaluación de cortocircuito significa que el código evaluará el mínimo que necesita para pasar o fallar una condición. Si el ejemplo # 1, si foo () devuelve falso, bar () y baz () ni siquiera se evalúan. Esto es útil si baz () es una llamada de función de larga duración porque puede omitirla si foo () devuelve falso o foo () devuelve verdadero y bar () devuelve falso.

Este no es el caso en el ejemplo # 2. foo (), bar () y baz () siempre se evalúan. Si espera que bar () y baz () exhiban efectos secundarios, tendrá problemas con el Ejemplo # 1. El ejemplo # 1 anterior es en realidad equivalente a esto:

Ej # 3:

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

Tenga en cuenta estas diferencias en su código al elegir entre los ejemplos 1 y 2.

usuario2023861
fuente
1

Ampliando un poco el tema de la legibilidad ...

Se me ocurren dos buenas razones para almacenar el resultado en una variable:

  1. Si va a usar la condición más de una vez, guardarla en una variable significa que solo tiene que llamar a la función una vez. (Esto supone que el valor almacenado sigue siendo válido; si necesita volver a probarlo, por supuesto, esto no se aplica).

  2. Si almacenarlo en una variable mejora la legibilidad al darle a la condición un nombre más significativo que el que se ve en la llamada a la función.

Por ejemplo, esto:

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

no ayuda a la legibilidad, pero esto:

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

probablemente sí, ya que no es obvio de inmediato que eso feof(file1) && feof(file2)significa que hemos terminado el procesamiento.

los passwordsMatch variable en la pregunta es probablemente un mejor ejemplo que el mío.

Las variables de un solo uso son útiles si dan un nombre significativo a algún valor. (Por supuesto, esto es para beneficio del lector humano).

Keith Thompson
fuente
Creo que prefiero dejar un comentario en lugar de presentar la done_processingvariable. Después de todo, el nombre done_processingcumple la función de un comentario. Y un comentario real me permite decir una o dos palabras sobre por qué esta condición indica que hemos terminado con el procesamiento. No es que sea un gran comentarista, sin embargo, probablemente no lo haría en código real ...
cmaster - reinstalar a monica el
@cmaster: Un problema con los comentarios es que pueden desincronizarse fácilmente con el código. Una variable denominada done_processinges una forma más duradera de expresar la intención.
Keith Thompson
+1 para el punto número 2: a veces una var temp bien nombrada realmente aclara las cosas.
user949300