Style de flux de contrôle avec contrôles de validation

27

Je me retrouve à écrire beaucoup de code comme ceci:

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

Cela peut devenir assez compliqué, surtout si plusieurs vérifications sont impliquées. Dans de tels cas, j'ai expérimenté d'autres styles, comme celui-ci:

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

Je suis intéressé par des commentaires sur les choix stylistiques ici. [Ne vous inquiétez pas trop des détails des déclarations individuelles; c'est le flux de contrôle global qui m'intéresse.]

William Jockusch
la source
8
Permettez-moi de souligner que vous avez une erreur de spécification assez grave dans votre exemple. Si, par exemple, l'actif == 42 et le passif == 43, vous déclarerez la personne inexistante.
John R. Strohm
Ne serait-il pas préférable de lever une exception et de laisser le code client gérer les validations?
Tulains Córdova
@ TulainsCórdova Des exceptions peuvent ne pas être disponibles, ou des données non valides ne sont peut-être pas assez exceptionnelles pour que l'impact sur les performances de la création de la trace de pile, etc. soit acceptable.
Hulk

Réponses:

27

Pour ce type de problème, Martin Fowler a proposé un modèle de spécification :

... modèle de conception, par lequel les règles métier peuvent être recombinées en enchaînant les règles métier en utilisant une logique booléenne.
 
Un modèle de spécification décrit une règle métier qui peut être combinée avec d'autres règles métier. Dans ce modèle, une unité de logique métier hérite ses fonctionnalités de la classe de spécification composite agrégée abstraite. La classe Spécification composite a une fonction appelée IsSatisfiedBy qui renvoie une valeur booléenne. Après l'instanciation, la spécification est "enchaînée" avec d'autres spécifications, ce qui rend la nouvelle logique métier facilement maintenable, mais hautement personnalisable. De plus, lors de l'instanciation, la logique métier peut, par invocation de méthode ou inversion de contrôle, voir son état modifié afin de devenir un délégué d'autres classes comme un référentiel de persistance ...

Les sons ci-dessus sont un peu élevés (du moins pour moi), mais quand je l'ai essayé dans mon code, cela s'est bien passé et s'est avéré facile à mettre en œuvre et à lire.

De mon point de vue, l'idée principale est d'extraire du code qui effectue les vérifications dans des méthodes / objets dédiés.

Avec votre netWorthexemple, cela pourrait ressembler à ceci:

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

Votre cas semble assez simple de sorte que toutes les vérifications semblent OK pour tenir dans une liste simple dans une seule méthode. Je dois souvent me séparer de plusieurs méthodes pour améliorer la lecture.

Je regroupe / extrait généralement des méthodes liées aux "spécifications" dans un objet dédié, bien que votre cas semble OK sans cela.

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

Cette question sur Stack Overflow recommande quelques liens en plus de celui mentionné ci-dessus: Exemple de modèle de spécification . En particulier, les réponses suggèrent Dimecasts «Learning the Specification pattern» pour une procédure pas à pas d'un exemple et mentionnent le document «Spécifications» rédigé par Eric Evans et Martin Fowler .

moucheron
la source
8

Je trouve plus facile de déplacer la validation vers sa propre fonction, cela aide à garder l'intention des autres fonctions plus propre, donc votre exemple serait comme ça.

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
la source
2
Pourquoi avez-vous l' ifentrée validPerson? Revenez simplement à la person!=NULL && person->isAlive && person->assets !=-1 && person->liabilities != -1place.
David Hammen
3

Une chose que j'ai particulièrement bien fonctionnée est l'introduction d'une couche de validation dans votre code. Vous avez d'abord une méthode qui effectue toute la validation désordonnée et renvoie des erreurs (comme -1dans vos exemples ci-dessus) en cas de problème. Une fois la validation terminée, la fonction appelle une autre fonction pour effectuer le travail réel. Maintenant, cette fonction n'a pas besoin de faire toutes ces étapes de validation car elles devraient déjà être effectuées. C'est-à-dire que la fonction de travail suppose que l'entrée est valide. Comment devez-vous gérer les hypothèses? Vous les affirmez dans le code.

Je pense que cela rend le code très facile à lire. La méthode de validation contient tout le code désordonné pour traiter les erreurs côté utilisateur. La méthode de travail documente proprement ses hypothèses avec des assertions et n'a alors pas à travailler avec des données potentiellement invalides.

Considérez cette refactorisation de votre exemple:

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
la source