Vérifier si une méthode retourne false: attribuer le résultat à une variable temporaire, ou mettre l'appel de méthode directement au conditionnel?

9

Est-ce une bonne pratique d'appeler une méthode qui renvoie des valeurs vraies ou fausses dans une instruction if?

Quelque chose comme ça:

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

ou est-il préférable de les stocker dans une variable puis de les comparer en utilisant des valeurs if else comme celle-ci

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

Je ne fais pas seulement référence à .NET, je fais référence à la question dans tous les langages de programmation, il se trouve que j'ai utilisé .NET comme exemple

KyelJmD
la source
3
Si vous utilisez une variable temporaire, écrivez if (!validate)plutôt que if (validate == false).
Philip
12
Je nommerais la fonction quelque chose comme "CredentialsAreValid ()" afin que vous sachiez qu'elle devrait renvoyer un bool mais sinon oui, sa bonne pratique
Zachary K
3
IsValidCredentials, bien que grammaticalement maladroit, est un format courant pour indiquer une valeur de retour booléenne.
zzzzBov
@Philip quelle est la différence entre if (! Validate) et this if (validate) ??
KyelJmD
!est l'opérateur "NOT", il annule toute expression booléenne. C'est if (!validate)le contraire de if (validate). L'instruction if sera entrée if validaten'est pas vraie.
Philip

Réponses:

26

Comme pour toutes ces choses, cela dépend.

Si vous n'utilisez pas le résultat de votre appel à, ValidateCredentialsil n'est pas nécessaire (sauf à des fins de débogage) de stocker le résultat dans une variable locale. Cependant, si cela rend le code plus lisible (et donc plus facile à maintenir), une variable va avec.

Le code ne sera pas sensiblement moins efficace.

ChrisF
la source
1
ce type de texte est-il lisible? comme celui que j'ai fait ci-dessus
KyelJmD
@KyelJmD: Cela dépend de la culture dans laquelle vous programmez. Aux endroits où j'ai programmé, le !ValidateCredentialsest plus lisible car il dit explicitement ce qu'il fait dans le code. C'est très clair. Si la fonction que vous appelez n'a pas de nom "auto-documenté", il vaut peut-être mieux utiliser la variable. Dans l'état actuel des choses, je recommanderais d'omettre la variable et de conserver le code d'auto-documentation que vous avez.
Joel Etherton
ValidateCredentials n'est pas lisible en premier lieu - comment le nom vous dit-il ce que signifie le résultat? Beaucoup mieux soit CredentialsAreValid ou CredentialsAreInvalid.
gnasher729
9

Pourquoi utiliser une variable supplémentaire? Je préfère utiliser la première approche, elle est plus lisible et simple.

Diego Pacheco
la source
4

Est-ce une bonne pratique d'appeler une méthode qui renvoie des valeurs vraies ou fausses dans une instruction if?

Oui, si le conditionnel n'est pas assez simple pour être intégré et qu'il soit lisible.

ou est-il préférable de les stocker dans une variable puis de les comparer en utilisant des valeurs if else comme celle-ci

Vous ne devez le faire que si vous utilisez la valeur à plusieurs endroits ou si vous en avez besoin pour rendre le code plus lisible. Sinon, l'affectation à une variable n'est pas nécessaire. Un code inutile est au mieux un gaspillage et au pire une source de défaut.

Dietbuddha
la source
2

Laisse voir...

Parce qu'il s'agit de KISS , il n'est pas nécessaire de créer une variable supplémentaire lorsque vous pouvez vous en passer. De plus, il n'est pas nécessaire de taper plus ... quand il n'y en a pas besoin.

Mais ensuite, parce que vous SECHEZ , si vous appelez plus tard ValidateCredentialset que vous vous retrouvez à taper, ValidateCredentials(txtUser.Text, txtPassword.Text)vous savez que vous devriez avoir créé une variable supplémentaire.

kowsheek
la source
2

Oui, il est généralement bon d'utiliser de telles méthodes comme si les conditions étaient réunies. Il est cependant utile si le nom de la méthode indique qu'elle renvoie un booléen; par exemple CanValidateCredentials. Dans les langages de style C, cette méthode prend souvent la forme de préfixes Is et Can, et en Ruby avec le "?" suffixe.

Christian Horsdal
la source
1
Bon point sur les noms de méthode, mais je ne commencerais jamais un nom de méthode avec "si". Ensuite, le code lit "si si". "Can" est ok: if (cat.canOpenCanOfCatFood()).
kevin cline
Merci, @Kevin. Je voulais dire «n'est» pas «si». J'ai édité la réponse
Christian Horsdal
1

Il y a une autre préoccupation qui n'a pas encore été soulignée: l' évaluation des courts-circuits . Quelle est la différence entre ces deux fragments de code?

Ex # 1:

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

Ex # 2:

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

Ces deux fragments de code semblent faire la même chose, mais si votre langue prend en charge l'évaluation des courts-circuits, ces deux fragments sont différents. L'évaluation de court-circuit signifie que le code évaluera le minimum dont il a besoin pour réussir ou échouer une condition. Si l'exemple # 1, si foo () renvoie false, bar () et baz () ne sont même pas évalués. Ceci est utile si baz () est un appel de fonction de longue durée car vous pouvez l'ignorer si foo () retourne false ou foo () retourne vrai et bar () retourne faux.

Ce n'est pas le cas dans l'exemple # 2. foo (), bar () et baz () sont toujours évalués. Si vous vous attendez à ce que bar () et baz () présentent des effets secondaires, vous aurez des problèmes avec l'exemple # 1. L'exemple # 1 ci-dessus est en fait équivalent à ceci:

Ex # 3:

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

Soyez conscient de ces différences dans votre code lorsque vous choisissez entre les exemples # 1 et # 2.

user2023861
la source
1

Développer un peu le problème de lisibilité ...

Je peux penser à deux bonnes raisons de stocker le résultat dans une variable:

  1. Si vous comptez utiliser la condition plusieurs fois, l'enregistrer dans une variable signifie que vous ne devez appeler la fonction qu'une seule fois. (Cela suppose que la valeur stockée est toujours valide; si vous devez la tester à nouveau, cela ne s'applique bien sûr pas.)

  2. Si le stockage dans une variable améliore la lisibilité en donnant à la condition un nom plus significatif que ce que vous voyez dans l'appel de fonction.

Par exemple, ceci:

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

n'aide pas la lisibilité, mais ceci:

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

le fait probablement, car ce n'est pas immédiatement évident, cela feof(file1) && feof(file2)signifie que nous avons terminé le traitement.

La passwordsMatchvariable dans la question est probablement un meilleur exemple que le mien.

Les variables à usage unique sont utiles si elles donnent un nom significatif à une valeur. (C'est bien sûr pour le bénéfice du lecteur humain.)

Keith Thompson
la source
Je pense que je préfère laisser un commentaire au lieu d'introduire la done_processingvariable. Après tout, le nom a done_processingla fonction d'un commentaire. Et un vrai commentaire me permet de dire un ou deux mots sur les raisons pour lesquelles cette condition signale que nous en avons fini avec le traitement. Non pas que je sois un grand commentateur, cependant, je ne ferais probablement ni l'un ni l'autre en vrai code ...
cmaster - réintègre monica
@cmaster: Un problème avec les commentaires est qu'ils peuvent très facilement se désynchroniser avec le code. Une variable nommée done_processingest un moyen plus durable d'exprimer l'intention.
Keith Thompson
+1 pour le point numéro 2 - parfois une variable temp bien nommée clarifie vraiment les choses.
user949300