Stil für den Kontrollfluss mit Validierungsprüfungen

27

Ich finde mich viel Code wie folgt zu schreiben:

int myFunction(Person* person) {
  int personIsValid = !(person==NULL);
  if (personIsValid) {
     // do some stuff; might be lengthy
     int myresult = whatever;
     return myResult;
  }
  else {
    return -1;
  }
}

Es kann ziemlich chaotisch werden, insbesondere wenn mehrere Überprüfungen beteiligt sind. In solchen Fällen habe ich mit alternativen Stilen experimentiert, wie zum Beispiel diesem:

int netWorth(Person* person) {
  if (Person==NULL) {
    return -1;
  }
  if (!(person->isAlive))  {
    return -1;
  }
  int assets = person->assets;
  if (assets==-1)  {
    return -1;
  }
  int liabilities = person->liabilities;
  if (liabilities==-1) {
    return -1;
  }
  return assets - liabilities;
}

Ich bin an Kommentaren zu den hier getroffenen stilistischen Entscheidungen interessiert. [Sorgen Sie sich nicht zu sehr um die Details einzelner Aussagen; Es ist der gesamte Kontrollfluss, der mich interessiert.]

William Jockusch
quelle
8
Lassen Sie mich darauf hinweisen, dass Ihr Beispiel einen ziemlich schwerwiegenden Spezifikationsfehler enthält. Wenn zum Beispiel Aktiva == 42 und Passiva == 43 sind, erklären Sie die Person für nicht existent.
John R. Strohm
Wäre es nicht besser, eine Ausnahme auszulösen und den Clientcode Validierungen verwalten zu lassen?
Tulains Córdova
@ TulainsCórdova Ausnahmen sind möglicherweise nicht verfügbar, oder ungültige Daten sind möglicherweise nicht außergewöhnlich genug, damit die Auswirkungen auf die Leistung beim Erstellen des Stack-Trace usw. akzeptabel sind.
Hulk

Antworten:

27

Für diese Art von Fragen schlug Martin Fowler das Spezifikationsmuster vor :

... Entwurfsmuster, bei dem Geschäftsregeln durch Verketten der Geschäftsregeln mithilfe der Booleschen Logik neu kombiniert werden können.
 
Ein Spezifikationsmuster beschreibt eine Geschäftsregel, die mit anderen Geschäftsregeln kombiniert werden kann. In diesem Muster erbt eine Business-Logik-Einheit ihre Funktionalität von der abstrakten aggregierten Klasse Composite Specification. Die Composite Specification-Klasse verfügt über eine Funktion namens IsSatisfiedBy, die einen booleschen Wert zurückgibt. Nach der Instantiierung wird die Spezifikation mit anderen Spezifikationen "verkettet", wodurch neue Spezifikationen leicht zu pflegen und dennoch in hohem Maße anpassbar sind. Darüber hinaus kann der Status der Geschäftslogik durch Methodenaufruf oder Umkehrung der Steuerung geändert werden, um ein Delegat anderer Klassen wie z. B. eines Persistenz-Repositorys zu werden.

Oben klingt ein bisschen hochnäsig (zumindest für mich), aber als ich es in meinem Code ausprobierte, lief es ziemlich reibungslos und erwies sich als einfach zu implementieren und zu lesen.

Nach meiner Auffassung besteht die Hauptidee darin, Code zu "extrahieren", der die Prüfungen in dedizierten Methoden / Objekten durchführt.

In Ihrem netWorthBeispiel könnte dies folgendermaßen aussehen:

int netWorth(Person* person) {
  if (isSatisfiedBySpec(person)) {
    return person->assets - person->liabilities;
  }
  log("person doesn't satisfy spec");
  return -1;
}

#define BOOLEAN int // assuming C here
BOOLEAN isSatisfiedBySpec(Person* person) {
  return Person != NULL
      && person->isAlive
      && person->assets != -1
      && person->liabilities != -1;
}

Ihr Fall scheint ziemlich einfach zu sein, sodass alle Überprüfungen so aussehen, dass sie in eine einfache Liste innerhalb einer einzelnen Methode passen. Ich muss oft auf mehrere Methoden aufteilen, um das Lesen zu verbessern.

Normalerweise gruppiere / extrahiere ich auch "spec" -bezogene Methoden in einem dedizierten Objekt, obwohl Ihr Fall ohne das in Ordnung aussieht.

  // ...
  Specification s, *spec = initialize(s, person);
  if (spec->isSatisfied()) {
    return person->assets - person->liabilities;
  }
  log("person doesn't satisfy spec");
  return -1;
  // ...

Diese Frage bei Stack Overflow empfiehlt einige Links zusätzlich zu den oben genannten: Spezifikationsmuster-Beispiel . Insbesondere schlagen die Antworten Dimecasts 'Learning the Specification pattern' für eine exemplarische Darstellung eines Beispiels vor und erwähnen das von Eric Evans und Martin Fowler verfasste "Specifications" -Papier .

Mücke
quelle
8

Ich finde es einfacher, die Validierung auf eine eigene Funktion zu verlagern. Sie trägt dazu bei, die Absichten anderer Funktionen klarer zu machen, sodass Ihr Beispiel so aussehen würde.

int netWorth(Person* person) { 
    if(validPerson(person)) {
        int assets = person->assets;
        int liabilities = person->liabilities;
        return assets - liabilities;
    }
    else {
        return -1;
    }
}

bool validPerson(Person* person) { 
    if(person!=NULL && person->isAlive
      && person->assets !=-1 && person->liabilities != -1)
        return true;
    else
        return false;
}
Ryathal
quelle
2
Warum hast du die ifin validPerson? Kehren Sie person!=NULL && person->isAlive && person->assets !=-1 && person->liabilities != -1stattdessen einfach zurück .
David Hammen
3

Eine Sache, die ich besonders gut gesehen habe, ist die Einführung einer Validierungsschicht in Ihren Code. Zuerst haben Sie eine Methode, die die gesamte fehlerhafte Überprüfung durchführt und Fehler zurückgibt (wie -1in Ihren obigen Beispielen), wenn etwas schief geht. Wenn die Validierung abgeschlossen ist, ruft die Funktion eine andere Funktion auf, um die eigentliche Arbeit auszuführen. Jetzt muss diese Funktion nicht alle diese Validierungsschritte ausführen, da sie bereits ausgeführt werden sollten. Das heißt, die Arbeitsfunktion nimmt an, dass die Eingabe gültig ist. Wie gehen Sie mit Annahmen um? Sie behaupten sie im Code.

Ich denke, das macht den Code sehr einfach zu lesen. Die Validierungsmethode enthält den gesamten chaotischen Code, um Fehler auf der Benutzerseite zu behandeln. Die Arbeitsmethode dokumentiert ihre Annahmen sauber mit Zusicherungen und muss dann nicht mit potenziell ungültigen Daten arbeiten.

Betrachten Sie dieses Refactoring Ihres Beispiels:

int myFunction(Person* person) {
  int personIsValid = !(person==NULL);
  if (personIsValid) {
     return myFunctionWork(person)
  }
  else {
    return -1;
  }
}

int myFunction(Person *person) {
  assert( person != NULL);  
  // Do work and return
}
Oleksi
quelle