Best Practice - Wrapping, wenn um Funktionsaufruf herum, vs Hinzufügen eines frühen Exits, wenn Guard in Funktion ist

9

Ich weiß, dass dies sehr anwendungsfallspezifisch sein kann, aber ich frage mich viel zu oft. Gibt es eine allgemein bevorzugte Syntax?

Ich frage nicht, was der beste Ansatz ist, wenn ich in einer Funktion bin. Ich frage, ob ich vorzeitig beenden oder die Funktion einfach nicht aufrufen soll.

Wrap, wenn um Funktionsaufruf


if (shouldThisRun) {
  runFunction();
}

Habe wenn ( Wache ) in Funktion

runFunction() {
  if (!shouldThisRun) return;
}

Die letztere Option hat offensichtlich das Potenzial, die Codeduplizierung zu reduzieren, wenn diese Funktion mehrmals aufgerufen wird. Manchmal fühlt es sich jedoch falsch an, sie hier hinzuzufügen, da Sie möglicherweise die Einzelverantwortung der Funktion verlieren .


Hier ist ein Beispiel

Wenn ich eine updateStatus () -Funktion habe, die einfach den Status von etwas aktualisiert. Ich möchte den Status nur aktualisieren, wenn sich der Status geändert hat. Ich kenne die Stellen in meinem Code, an denen sich der Status ändern kann, und ich kenne andere Stellen, an denen er sich trotzig geändert hat.

Ich bin mir nicht sicher, ob es nur ich ist, aber es fühlt sich etwas schmutzig an, diese Innenfunktion zu überprüfen, da ich diese Funktion so rein wie möglich halten möchte - wenn ich sie aufrufe, erwarte ich, dass der Status aktualisiert wird. Aber ich kann nicht sagen, ob es besser ist, den Anruf an den wenigen Stellen, an denen ich weiß, dass er sich möglicherweise nicht geändert hat, in einen Scheck zu packen.

Matthew Mullin
quelle
3
@gnat Nein, diese Frage lautet im Wesentlichen "Was ist die bevorzugte Syntax bei einem frühen Exit", während meine Frage "Soll ich vorzeitig beenden oder sollte ich die Funktion einfach nicht aufrufen" lautet
Matthew Mullin
4
Sie können nicht darauf vertrauen, dass Entwickler, auch Sie selbst , die Vorbedingungen der Funktion überall dort, wo sie aufgerufen wird, korrekt überprüfen. Aus diesem Grund würde ich vorschlagen, dass die Funktion alle erforderlichen Bedingungen intern überprüft, wenn sie dazu in der Lage ist.
TZHX
"Ich möchte nur, dass der Status aktualisiert wird, wenn sich der Status geändert hat" - Sie möchten, dass ein Status aktualisiert (= geändert) wird, wenn sich der gleiche Status geändert hat? Klingt ziemlich kreisförmig. Können Sie bitte klarstellen, was Sie damit meinen, damit ich meiner Antwort darauf ein aussagekräftiges Beispiel hinzufügen kann?
Doc Brown
@DocBrown Nehmen wir zum Beispiel an, ich möchte zwei verschiedene Eigenschaften des Objektstatus synchron halten. Wenn sich eines der Objekte ändert, rufe ich syncStatuses () auf - dies kann jedoch für viele verschiedene Feldänderungen ausgelöst werden (nicht nur für das Statusfeld).
Matthew Mullin

Antworten:

15

Umschließen eines Funktionsaufrufs:
Dies dient dazu, zu entscheiden, ob die Funktion überhaupt aufgerufen werden soll, und ist Teil des Entscheidungsprozesses Ihres Programms.

Guard-Klausel in Funktion (Early Return):
Dies dient zum Schutz vor dem Aufruf mit ungültigen Parametern

Eine auf diese Weise verwendete Schutzklausel hält die Funktion "rein" (Ihr Begriff). Es ist nur vorhanden, um sicherzustellen, dass die Funktion nicht mit fehlerhaften Eingabedaten unterbrochen wird.

Die Logik, ob die Funktion überhaupt aufgerufen werden soll, liegt auf einer höheren Abstraktionsebene, wenn auch nur gerecht. Es sollte außerhalb der Funktion selbst existieren. Wie DocBrown sagt, können Sie eine Zwischenfunktion haben, die diese Prüfung durchführt, um den Code zu vereinfachen.

Dies ist eine gute Frage, die zu den Fragen gehört, die zum Erkennen von Abstraktionsebenen führen. Jede Funktion sollte auf einer einzelnen Abstraktionsebene arbeiten, und sowohl die Programmlogik als auch die Funktionslogik in der Funktion zu haben, fühlt sich für Sie falsch an - dies liegt daran, dass sie sich auf verschiedenen Abstraktionsebenen befinden. Die Funktion selbst ist eine niedrigere Ebene.

Indem Sie diese getrennt halten, stellen Sie sicher, dass Ihr Code einfacher zu schreiben, zu lesen und zu warten ist.

Baldrickk
quelle
Tolle Antwort. Ich mag die Tatsache, dass es mir eine klare Möglichkeit gibt, darüber nachzudenken. Das if sollte außerhalb sein, da es "Teil des Entscheidungsprozesses" ist, ob die Funktion überhaupt aufgerufen werden soll. Und hat von Natur aus nichts mit der Funktion selbst zu tun. Es fühlt sich komisch an, eine Meinungsantwort als richtig zu markieren, aber ich werde in ein paar Stunden noch einmal nachsehen und dies tun.
Matthew Mullin
Wenn es hilft, betrachte ich dies nicht als "Meinungs" -Antwort. Ich stelle fest, dass es sich falsch "anfühlt", aber das liegt daran, dass die verschiedenen Abstraktionsebenen nicht getrennt sind. Was ich aus Ihrer Frage erhalten habe, ist, dass Sie sehen können, dass es nicht „richtig“ ist, aber weil Sie nicht über Abstraktionsebenen nachdenken, ist es schwer zu quantifizieren, daher ist es etwas, das Sie nur schwer in Worte fassen können.
Baldrickk
7

Sie können beides haben - eine Funktion, die keine Parameter überprüft, und eine andere, die dies tut (möglicherweise einige Informationen darüber zurückgeben, ob der Aufruf ausgeführt wurde):

bool tryRunFunction(...)
{
    bool shouldThisRun = /* some logic using data not available inside "runFunction"*/;
    if (shouldThisRun)
        runFunction();
    return shouldThisRun;
}

Auf diese Weise können Sie doppelte Logik vermeiden, indem Sie eine wiederverwendbare tryRunFunctionFunktion bereitstellen und trotzdem Ihre ursprüngliche (möglicherweise reine) Funktion verwenden, die die Prüfung nicht im Inneren durchführt.

Beachten Sie, dass Sie manchmal eine Funktion wie tryRunFunctionbei einem integrierten Scheck ausschließlich benötigen , damit Sie den Scheck in integrieren können runFunction. Oder Sie müssen die Prüfung an keiner Stelle in Ihrem Programm erneut verwenden. In diesem Fall können Sie sie in der aufrufenden Funktion belassen.

Versuchen Sie jedoch, dem Aufrufer transparent zu machen, was passiert, indem Sie Ihren Funktionen Eigennamen geben. Aufrufer müssen also nicht raten oder in die Implementierung schauen, wenn sie die Überprüfungen selbst durchführen müssen oder wenn die aufgerufene Funktion dies bereits für sie tut. Ein einfaches Präfix wie trykann hierfür oft ausreichen.

Doc Brown
quelle
1
Ich muss zugeben, dass die Redewendung "tryXXX ()" immer ein bisschen anders schien und hier unangemessen ist. Sie versuchen nicht, etwas zu tun, das einen wahrscheinlichen Fehler erwartet. Sie aktualisieren, wenn es schmutzig ist.
user949300
@ user949300: Die Auswahl eines guten Namens oder Namensschemas hängt vom tatsächlichen Anwendungsfall, den tatsächlichen Funktionsnamen und nicht von einem erfundenen Namen ab runFunction. Eine Funktion wie updateStatus()kann von einer anderen Funktion wie begleitet sein updateIfStatusHasChanged(). Dies ist jedoch zu 100% fallabhängig. Es gibt keine "One-Size-Fits-All" -Lösung. Ja, ich stimme zu, die "Try" -Sprache ist nicht immer eine gute Wahl.
Doc Brown
Es gibt nichts namens "dryRun"? Mehr oder weniger wird eine regelmäßige Ausführung ohne Nebenwirkungen. Wie man die Nebenwirkungen deaktiviert, ist eine andere Geschichte
Laiv
3

Wer entscheidet, ob er läuft, lautet die Antwort von GRASP , wer der "Informationsexperte" ist, der es weiß.

Wenn Sie sich dazu entschlossen haben, sollten Sie die Funktion aus Gründen der Übersichtlichkeit umbenennen.

So etwas, wenn die Funktion entscheidet:

 ensureUpdated()
 updateIfDirty()

Oder wenn der Anrufer entscheiden soll:

 writeStatus()
user949300
quelle
2

Ich möchte die Antwort von @ Baldrickk erweitern.

Es gibt keine allgemeine Antwort auf Ihre Frage. Dies hängt von der Bedeutung (Vertrag) der aufzurufenden Funktion und der Art der Bedingung ab.

Lassen Sie es uns im Kontext Ihres Beispielaufrufs diskutieren updateStatus(). Der Vertrag besteht wahrscheinlich darin, einen Status zu aktualisieren, da etwas passiert ist, das Einfluss auf den Status hat. Ich würde erwarten, dass Aufrufe dieser Methode zulässig sind, auch wenn es keine echte Statusänderung gibt, und notwendig, wenn es eine echte Änderung gibt.

Eine anrufende Site kann also einen Anruf überspringen, updateStatus()wenn sie weiß, dass (innerhalb ihres Domänenhorizonts) nichts Relevantes geändert wurde. In diesen Situationen sollte der Aufruf von einem geeigneten ifKonstrukt umgeben sein.

Innerhalb der updateStatus()Funktion kann es Situationen geben, in denen diese Funktion (anhand von Daten innerhalb ihres Domänenhorizonts) erkennt, dass nichts zu tun ist, und dort sollte sie frühzeitig zurückkehren.

Die Fragen sind also:

  • Wann darf / muss die Funktion von außen unter Berücksichtigung des Vertrags der Funktion aufgerufen werden?
  • Gibt es innerhalb der Funktion Situationen, in denen sie ohne echte Arbeit frühzeitig zurückkehren kann?
  • Gehört die Bedingung, ob die Funktion aufgerufen werden soll oder ob vorzeitig zurückgekehrt werden soll, zur internen Domäne der Funktion oder nach außen?

Mit einer updateStatus()Funktion würde ich erwarten, beide zu sehen, Sites aufzurufen, die wissen, dass sich nichts geändert hat, den Aufruf zu überspringen und die Implementierung frühzeitig auf "nichts geänderte" Situationen zu prüfen, selbst wenn auf diese Weise dieselbe Bedingung manchmal zweimal überprüft wird, beide innen und außen.

Ralf Kleberhoff
quelle
2

Es gibt viele gute Erklärungen. Aber ich möchte ungewöhnlich aussehen: Angenommen, Sie verwenden diesen Weg:

if (shouldThisRun) {
   runFunction();
}

runFunction() {
   if (!shouldThisRun) return;
}

Und Sie müssen eine andere Funktion in einer runFunctionMethode wie dieser aufrufen :

runFunction() {
   if (!shouldThisRun) return;
   someOtherfunction();
}

Was wirst du tun? Kopieren Sie alle Validierungen von oben nach unten?

someOtherfunction() {
   if (!shouldThisRun) return;
}

Das glaube ich nicht. Daher mache ich normalerweise den gleichen Ansatz: Eingaben validieren und Bedingungen in der publicMethode überprüfen . Öffentliche Methoden sollten ihre eigenen Validierungen durchführen und die erforderlichen Bedingungen überprüfen, die selbst Anrufer erfüllen. Aber lassen Sie private Methoden einfach ihr eigenes Geschäft machen . Eine andere Funktion wird möglicherweise aufgerufen, runFunctionohne eine Validierung durchzuführen oder eine Bedingung zu überprüfen.

public someFunction() {
   if (shouldThisRun) {
      runFunction();
   }
}

private runFunction() {
 // do your business.
}
Engineert
quelle