Meilleure pratique - Habillage si autour de l'appel de fonction vs Ajout d'une sortie anticipée si le garde est en fonction

Je sais que cela peut être très spécifique au cas d'utilisation, mais je me pose trop souvent la question. Existe-t-il une syntaxe généralement préférée.

Je ne demande pas quelle est la meilleure approche quand dans une fonction, je demande si je dois quitter tôt ou si je n'appelle simplement pas la fonction.

Envelopper si autour de l'appel de fonction

if (shouldThisRun) {
  runFunction();
}

Avoir si ( garde ) en fonction

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

Cette dernière option a évidemment le potentiel de réduire la duplication de code si cette fonction est appelée plusieurs fois, mais il est parfois mal de l'ajouter ici, car vous risquez de perdre la responsabilité unique de la fonction.

Voici un exemple

Si j'ai une fonction updateStatus () qui met simplement à jour l'état de quelque chose. Je veux seulement que le statut soit mis à jour si le statut a changé. Je connais les endroits de mon code où le statut a le potentiel de changer, et je connais d'autres endroits où il a changé de défi.

Je ne suis pas sûr que ce soit juste moi, mais cela semble un peu sale de vérifier cette fonction interne car je veux garder cette fonction aussi pure que possible - si je l'appelle, je m'attends à ce que le statut soit mis à jour. Mais je ne peux pas dire s'il vaut mieux envelopper l'appel dans un chèque les quelques endroits où je sais qu'il a le potentiel de ne pas avoir changé.

Envelopper un if autour d'un appel de fonction:
Ceci permet de décider si la fonction doit être appelée du tout, et fait partie du processus de prise de décision de votre programme.

Clause de garde en fonction (retour anticipé):
c'est pour éviter d'être appelé avec des paramètres invalides

Une clause de garde utilisée de cette manière conserve la fonction "pure" (votre terme). Il existe uniquement pour garantir que la fonction ne rompt pas avec des données d'entrée erronées.

La logique d'appeler ou non la fonction se situe à un niveau d'abstraction plus élevé, même juste. Il doit exister en dehors de la fonction elle-même. Comme le dit DocBrown, vous pouvez avoir une fonction intermédiaire qui effectue cette vérification, pour simplifier le code.

C'est une bonne question à poser et elle fait partie de l'ensemble des questions qui conduisent à reconnaître les niveaux d'abstraction. Chaque fonction doit fonctionner à un seul niveau d'abstraction, et le fait d'avoir à la fois la logique du programme et la logique de fonction dans la fonction vous semble mal - c'est parce qu'ils sont à différents niveaux d'abstraction. La fonction elle-même est un niveau inférieur.

En les conservant séparément, vous vous assurez que votre code sera plus facile à écrire, à lire et à maintenir.

Vous pouvez avoir les deux - une fonction qui ne vérifie pas les paramètres, et une autre qui le fait, comme ceci (peut-être en retournant des informations sur si l'appel a été fait):

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

De cette façon, vous pouvez éviter la logique en double en fournissant une fonction réutilisable tryRunFunctionet avoir toujours votre fonction d'origine (peut-être pure) qui ne fait pas le contrôle à l'intérieur.

Notez que parfois vous aurez besoin d'une fonction comme tryRunFunctionavec un chèque intégré exclusivement, afin que vous puissiez intégrer le chèque dans runFunction. Ou vous n'avez pas besoin de réutiliser le chèque n'importe où dans votre programme, auquel cas vous pouvez le laisser rester dans la fonction d'appel.

Cependant, essayez de rendre transparent pour l'appelant ce qui se passe en donnant à vos fonctions des noms propres. Ainsi, les appelants n'ont pas à deviner ou à examiner l'implémentation s'ils doivent effectuer eux-mêmes les vérifications ou si la fonction appelée le fait déjà pour eux. Un simple préfixe comme trypeut souvent suffire pour cela.

Quant à savoir qui décide de se présenter, la réponse est, du GRASP , qui est "l'expert en information" qui sait.

Une fois que vous avez décidé cela, envisagez de renommer la fonction pour plus de clarté.

Quelque chose comme ça, si la fonction décide:

 ensureUpdated()
 updateIfDirty()

Ou, si l'appelant est censé décider:

 writeStatus()

Je voudrais développer la réponse de @ Baldrickk.

Il n'y a pas de réponse générale à votre question. Cela dépend de la signification (contrat) de la fonction à appeler et de la nature de la condition.

Discutons-en donc dans le contexte de votre exemple d'appel updateStatus(). Son contrat est probablement de mettre à jour un statut car quelque chose ayant une influence sur le statut s'est produit. Je m'attendrais à ce que les appels à cette méthode soient autorisés même s'il n'y a pas de véritable changement de statut, et qu'ils soient nécessaires s'il y a un vrai changement.

Ainsi, un site appelant peut ignorer un appel updateStatus()s'il sait que (à l'intérieur de son horizon de domaine) rien de pertinent n'a changé. Ce sont les situations où l'appel doit être entouré d'une ifconstruction appropriée .

À l'intérieur de la updateStatus()fonction, il peut y avoir des situations où cette fonction détecte (à partir de données à l'intérieur de son horizon de domaine) qu'il n'y a rien à faire, et c'est là qu'elle devrait revenir tôt.

Donc, les questions sont:

• De l'extérieur, quand est-il autorisé / requis d'appeler la fonction, en tenant compte du contrat de la fonction?
• À l'intérieur de la fonction, y a-t-il des situations où elle peut revenir tôt sans travail réel?
• La condition d'appeler la fonction / de revenir tôt appartient-elle au domaine interne de la fonction ou à l'extérieur?

Avec une updateStatus()fonction, je m'attendrais à voir les deux, appeler des sites qui ne savent rien a changé, ignorer l'appel, et l'implémentation vérifie tôt les situations "rien n'a changé", même si de cette façon la même condition est parfois vérifiée deux fois, les deux à l'intérieur et à l'extérieur.

Il existe de nombreuses bonnes explications. Mais je veux regarder de manière inhabituelle: Supposons que vous utilisez de cette façon:

if (shouldThisRun) {
   runFunction();
}

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

Et vous devez appeler une autre fonction dans une runFunctionméthode comme celle-ci:

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

Que vas-tu faire? Copiez-vous toutes les validations de haut en bas?

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

Je ne pense pas. Ainsi, je fais généralement la même approche: valider les entrées et vérifier les conditions dans la publicméthode. Les méthodes publiques doivent effectuer leurs propres validations et vérifier les conditions requises, même les appelants. Mais laissez les méthodes privées faire leur propre affaire . Une autre fonction peut appeler runFunctionsans faire de validation ni vérifier aucune condition.

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

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