Überprüfen, ob eine Methode false zurückgibt: Ergebnis einer temporären Variablen zuweisen oder Methodenaufruf direkt in eine Bedingung setzen?

9

Ist es eine gute Praxis, eine Methode aufzurufen, die in einer if-Anweisung wahre oder falsche Werte zurückgibt?

Etwas wie das:

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

Oder ist es besser, sie in einer Variablen zu speichern, als sie mit solchen Werten zu vergleichen?

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

Ich beziehe mich nicht nur auf .NET, ich beziehe mich auf die Frage in allen Programmiersprachen. Es kommt einfach so vor, dass ich .NET als Beispiel verwendet habe

KyelJmD
quelle
3
Wenn Sie eine temporäre Variable verwenden, schreiben Sie if (!validate)statt if (validate == false).
Philip
12
Ich würde die Funktion so etwas wie "CredentialsAreValid ()" nennen, damit Sie wissen, dass sie einen Bool zurückgeben sollte, aber ansonsten ist es ja eine gute Praxis
Zachary K
3
IsValidCredentials, obwohl grammatikalisch umständlich, ist ein gängiges Format zur Angabe eines booleschen Rückgabewerts.
zzzzBov
@Philip was ist der Unterschied zwischen if (! Validate) und diesem if (validate) ??
KyelJmD
!ist der Operator "NOT" und negiert jeden booleschen Ausdruck. So if (!validate)ist das Gegenteil von if (validate). Die if-Anweisung wird eingegeben, wenn sie validatenicht wahr ist.
Philip

Antworten:

26

Wie bei all diesen Dingen kommt es darauf an.

Wenn Sie das Ergebnis Ihres Aufrufs nicht verwenden möchten, müssen Sie das Ergebnis ValidateCredentials(außer zu Debugging-Zwecken) nicht in einer lokalen Variablen speichern. Wenn es jedoch den Code lesbarer (und damit wartbarer) macht, muss eine Variable dazu passen.

Der Code wird nicht messbar weniger effizient sein.

ChrisF
quelle
1
ist diese Art von lesbar? wie die, die ich oben gemacht habe
KyelJmD
@KyelJmD: Das hängt von der Kultur ab, in der Sie programmieren. An den Stellen, an denen ich programmiert habe, !ValidateCredentialsist das besser lesbar, da es explizit angibt, was es im Code tut. Es ist sehr klar. Wenn die von Ihnen aufgerufene Funktion keinen "selbstdokumentierenden" Namen hat, ist es möglicherweise besser, mit der Variablen zu arbeiten. So wie es aussieht, würde ich empfehlen, die Variable wegzulassen und bei dem selbstdokumentierenden Code zu bleiben, den Sie haben.
Joel Etherton
ValidateCredentials ist überhaupt nicht lesbar - wie sagt Ihnen der Name, was das Ergebnis bedeutet? Viel besser, entweder CredentialsAreValid oder CredentialsAreInvalid.
Gnasher729
9

Warum eine zusätzliche Variable verwenden? Ich bevorzuge den ersten Ansatz, er ist lesbarer und einfacher.

Diego Pacheco
quelle
4

Ist es eine gute Praxis, eine Methode aufzurufen, die in einer if-Anweisung wahre oder falsche Werte zurückgibt?

Ja, wenn die Bedingung nicht einfach genug ist, um inline zu sein und lesbar zu sein.

Oder ist es besser, sie in einer Variablen zu speichern, als sie mit solchen Werten zu vergleichen?

Sie sollten dies nur tun, wenn Sie den Wert an mehreren Stellen verwenden oder benötigen, um den Code besser lesbar zu machen. Andernfalls ist die Zuordnung zu einer Variablen nicht erforderlich. Unnötiger Code ist bestenfalls verschwenderisch und im schlimmsten Fall eine Fehlerquelle.

Dietbuddha
quelle
2

Mal sehen ...

Da es nur um KISS geht , müssen Sie keine zusätzliche Variable erstellen, wenn Sie darauf verzichten können. Es ist auch nicht nötig, mehr zu tippen ... wenn es nicht nötig ist.

Aber weil Sie TROCKEN , wissen Sie, dass Sie eine zusätzliche Variable hätten erstellen müssen , wenn Sie später angerufen haben ValidateCredentialsund sich beim Tippen befinden ValidateCredentials(txtUser.Text, txtPassword.Text).

Aredkid
quelle
2

Ja, es ist normalerweise in Ordnung, solche Methoden als Bedingungen zu verwenden. Es ist jedoch hilfreich, wenn der Methodenname angibt, dass die Methode einen Bool zurückgibt. Zum Beispiel CanValidateCredentials. In Sprachen im C-Stil wird diese Methode häufig in Form von Is- und Can-Präfixen und in Ruby mit dem '?' Suffix.

Christian Horsdal
quelle
1
Guter Punkt zu Methodennamen, aber ich würde niemals einen Methodennamen mit "if" beginnen. Dann lautet der Code "if if". "Can" ist ok : if (cat.canOpenCanOfCatFood()).
Kevin Cline
Danke, @ Kevin. Ich meinte "Ist" nicht "Wenn". Ich habe die Antwort bearbeitet
Christian Horsdal
1

Es gibt noch ein weiteres Problem, auf das noch nicht hingewiesen wurde: die Kurzschlussbewertung . Was ist der Unterschied zwischen diesen beiden Codefragmenten?

Ex # 1:

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

Ex # 2:

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

Diese beiden Codefragmente scheinen dasselbe zu tun, aber wenn Ihre Sprache die Kurzschlussauswertung unterstützt, sind diese beiden Fragmente unterschiedlich. Kurzschlussauswertung bedeutet, dass der Code das Minimum auswertet, das erforderlich ist, um eine Bedingung zu bestehen oder nicht zu bestehen. Wenn Beispiel 1, wenn foo () false zurückgibt, werden bar () und baz () nicht einmal ausgewertet. Dies ist nützlich, wenn baz () ein lang laufender Funktionsaufruf ist, da Sie ihn überspringen können, wenn entweder foo () false oder foo () true und bar () false zurückgibt.

Dies ist in Beispiel 2 nicht der Fall. foo (), bar () und baz () werden immer ausgewertet. Wenn Sie erwarten, dass bar () und baz () Nebenwirkungen aufweisen, haben Sie Probleme mit Beispiel 1. Das obige Beispiel Nr. 1 entspricht tatsächlich diesem:

Ex # 3:

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

Beachten Sie diese Unterschiede in Ihrem Code, wenn Sie zwischen den Beispielen 1 und 2 wählen.

user2023861
quelle
1

Das Problem der Lesbarkeit etwas erweitern ...

Ich kann mir zwei gute Gründe vorstellen, das Ergebnis in einer Variablen zu speichern:

  1. Wenn Sie die Bedingung mehrmals verwenden, bedeutet das Speichern in einer Variablen, dass Sie die Funktion nur einmal aufrufen müssen. (Dies setzt voraus, dass der gespeicherte Wert noch gültig ist. Wenn Sie ihn erneut testen müssen, gilt dies natürlich nicht.)

  2. Wenn das Speichern in einer Variablen die Lesbarkeit verbessert, indem Sie der Bedingung einen Namen geben, der aussagekräftiger ist als das, was Sie im Funktionsaufruf sehen.

Zum Beispiel:

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

hilft nicht bei der Lesbarkeit, aber dies:

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

wahrscheinlich, da es nicht sofort offensichtlich ist, feof(file1) && feof(file2)dass wir mit der Verarbeitung fertig sind.

Die passwordsMatchVariable in der Frage ist wahrscheinlich ein besseres Beispiel als meine.

Einwegvariablen sind nützlich, wenn sie einem Wert einen aussagekräftigen Namen geben. (Dies ist natürlich zum Nutzen des menschlichen Lesers.)

Keith Thompson
quelle
Ich denke, ich würde lieber einen Kommentar hinterlassen, anstatt die done_processingVariable einzuführen . Immerhin hat der Name done_processingdie Funktion eines Kommentars. Und ein echter Kommentar erlaubt mir, ein oder zwei Worte darüber zu sagen, warum diese Bedingung signalisiert, dass wir mit der Verarbeitung fertig sind. Nicht, dass ich ein großartiger Kommentator wäre, aber ich würde wahrscheinlich auch nichts in echtem Code tun ...
cmaster -
@cmaster: Ein Problem mit Kommentaren ist, dass sie sehr leicht nicht mehr mit dem Code synchronisiert werden können. Eine benannte Variable done_processingist eine dauerhaftere Möglichkeit, die Absicht auszudrücken.
Keith Thompson
+1 für Punkt 2 - manchmal klärt eine gut benannte temporäre Variable die Dinge wirklich.
user949300