Mieux vaut avoir 2 méthodes au sens clair, ou juste 1 méthode à double usage?

30

Pour simplifier l'interface, vaut-il mieux ne pas avoir la getBalance()méthode? Passer 0à la charge(float c);volonté donnera le même résultat:

public class Client {
    private float bal;
    float getBalance() { return bal; }
    float charge(float c) {
        bal -= c;
        return bal;
    }
}

Peut-être faire une note javadoc? Ou, laissez-le simplement à l'utilisateur de la classe le soin de trouver un équilibre?

David
la source
26
Je suppose généreusement qu'il s'agit d'un exemple, d'un code illustratif et que vous n'utilisez pas de mathématiques à virgule flottante pour stocker des valeurs monétaires. Sinon, je devrais être tout prédicateur. :-)
corsiKa
1
@corsiKa nah. Ce n'est qu'un exemple. Mais, oui, j'aurais utilisé float pour représenter l'argent dans une vraie classe ... Merci de me rappeler les dangers des nombres à virgule flottante.
david
10
Certaines langues font la distinction entre les mutateurs et les accesseurs à bon escient. Il serait très ennuyeux de ne pas pouvoir obtenir l'équilibre d'une instance qui n'est accessible qu'en tant que constante!
JDługosz

Réponses:

90

Vous semblez suggérer que la complexité d'une interface est mesurée par le nombre d'éléments qu'elle possède (méthodes, dans ce cas). Beaucoup diront que le fait de devoir se rappeler que la chargeméthode peut être utilisée pour renvoyer l'équilibre d'un Clientajoute beaucoup plus de complexité que d'avoir l'élément supplémentaire de la getBalanceméthode. Rendre les choses plus explicites est beaucoup plus simple, surtout au point où cela ne laisse aucune ambiguïté, quel que soit le nombre plus élevé d'éléments dans l'interface.

En outre, appeler charge(0)viole le principe du moindre étonnement , également connu sous le nom de métrique WTF par minute (de Clean Code, image ci-dessous), rendant difficile pour les nouveaux membres de l'équipe (ou les membres actuels, après un certain temps loin du code) jusqu'à ils comprennent que l'appel est effectivement utilisé pour obtenir l'équilibre. Pensez à la réaction des autres lecteurs:

entrez la description de l'image ici

En outre, la signature de la chargeméthode va à l'encontre des directives de faire une et une seule chose et de séparer les requêtes de commande , car elle fait changer l'objet à son état tout en renvoyant une nouvelle valeur.

Dans l'ensemble, je pense que l'interface la plus simple dans ce cas serait:

public class Client {
  private float bal;
  float getBalance() { return bal; }
  void charge(float c) { bal -= c; }
}
MichelHenrich
la source
25
Le point sur la séparation commande-requête est extrêmement important, OMI. Imaginez que vous êtes un nouveau développeur sur ce projet. En parcourant le code, il est immédiatement clair que getBalance()renvoie une valeur et ne modifie rien. D'un autre côté, il charge(0)semble que cela modifie probablement quelque chose ... peut-être qu'il envoie un événement PropertyChanged? Et que renvoie-t-elle, la valeur précédente ou la nouvelle valeur? Maintenant, vous devez le rechercher dans les documents et gaspiller le cerveau, ce qui aurait pu être évité en utilisant une méthode plus claire.
Mage Xy
19
En outre, l'utilisation charge(0)comme moyen d'obtenir l'équilibre, car il se trouve qu'il n'a aucun effet, vous associe désormais à ce détail d'implémentation; Je peux facilement imaginer une future exigence où le suivi du nombre de charges deviendra pertinent, à quel point avoir votre base de code jonchée de charges qui ne sont pas vraiment des charges devient un point difficile. Vous devez fournir des éléments d'interface qui signifient les choses que vos clients doivent faire, pas ceux qui se contentent de faire les choses que vos clients doivent faire.
Ben
12
L'avertissement de CQS n'est pas nécessairement approprié. Pour une charge()méthode, si cette méthode est atomique dans un système simultané, il peut être approprié de renvoyer la nouvelle ou l'ancienne valeur.
Dietrich Epp
3
J'ai trouvé vos deux citations pour le CQRS extrêmement peu convaincantes. Par exemple, j'aime généralement les idées de Meyer, mais regarder le type de retour d'une fonction pour savoir si elle mute quelque chose est arbitraire et peu fiable. De nombreuses structures de données simultanées ou immuables nécessitent une mutation + des retours. Comment ajouteriez-vous à une chaîne sans violer CQRS? Avez-vous de meilleures citations?
user949300
6
Presque aucun code n'est conforme à la séparation des requêtes de commande. Il n'est pas idiomatique dans les langages orientés objet les plus répandus et est violé par les API intégrées de chaque langage que j'ai jamais utilisé. La décrire comme une "meilleure pratique" semble donc bizarre; pouvez-vous nommer un seul projet logiciel, jamais, qui suit réellement ce modèle?
Mark Amery
28

IMO, le remplacement getBalance()par l' charge(0)ensemble de votre application n'est pas une simplification. Oui, il y a moins de lignes, mais cela masque la signification de la charge()méthode, ce qui pourrait potentiellement causer des maux de tête sur la ligne lorsque vous ou quelqu'un d'autre devez revoir ce code.

Bien qu'ils puissent donner le même résultat, obtenir le solde d'un compte n'est pas la même chose que des frais de zéro, il serait donc préférable de séparer vos préoccupations. Par exemple, si vous avez déjà eu besoin de modifier charge()pour vous connecter chaque fois qu'il y a une transaction de compte, vous avez maintenant un problème et vous devrez quand même séparer la fonctionnalité.

Matt Dalzell
la source
13

Il est important de se rappeler que votre code doit être auto-documenté. Lorsque j'appelle charge(x), je m'attends xà être facturé. Les informations sur l'équilibre sont secondaires. De plus, je ne sais pas comment charge()est implémenté quand je l'appelle et je ne saurai certainement pas comment il sera implémenté demain. Par exemple, considérez cette future mise à jour potentielle pour charge():

float charge(float c) {
    lockDownUserAccountUntilChargeClears();
    bal -= c;
    Unlock();
    return bal;
}

Tout d'un coup, utiliser charge()pour obtenir l'équilibre n'est pas si bon.

David dit de réintégrer Monica
la source
Oui! Un bon point qui chargeest beaucoup plus lourd.
Déduplicateur
2

Utiliser charge(0);pour obtenir le solde est une mauvaise idée: un jour, quelqu'un pourrait y ajouter du code pour enregistrer les frais en cours sans réaliser l'autre utilisation de la fonction, puis chaque fois que quelqu'un obtient le solde, il sera enregistré en tant que charge. (Il existe des moyens de contourner cela, comme une déclaration conditionnelle qui dit quelque chose comme:

if (c > 0) {
    // make and log charge
}
return bal;

mais ceux-ci dépendent du programmeur sachant les implémenter, ce qu'il ne fera pas s'il n'est pas immédiatement évident qu'ils sont nécessaires.

En bref: ne comptez pas sur vos utilisateurs ou sur vos successeurs programmeurs pour réaliser que charge(0);c'est la bonne façon d'obtenir l'équilibre, car à moins qu'il n'y ait de la documentation qu'ils ne manqueront pas, alors franchement, cela ressemble à la façon la plus effrayante d'obtenir l'équilibre possible.

Micheal Johnson
la source
0

Je sais qu'il y a beaucoup de réponses, mais une autre raison contre charge(0)est qu'une simple faute de frappe charge(9)entraînera une diminution du solde de votre client chaque fois que vous souhaitez obtenir son solde. Si vous avez de bons tests unitaires, vous pourrez peut-être atténuer ce risque, mais si vous ne faites pas preuve de diligence à chaque appel, chargevous pourriez avoir cet incident.

Shaz
la source
-2

Je voudrais mentionner un cas particulier où il serait logique d'avoir moins de méthodes, plus polyvalentes: s'il y a beaucoup de polymorphisme, c'est-à-dire de nombreuses implémentations de cette interface ; surtout si ces implémentations sont dans du code développé séparément qui ne peut pas être mis à jour en synchronisation (l'interface est définie par une bibliothèque).

Dans ce cas, simplifier le travail d'écriture de chaque implémentation est beaucoup plus précieux que la clarté de son utilisation, car le premier évite les bogues de violation de contrat (les deux méthodes étant incompatibles), tandis que le second nuit seulement à la lisibilité, ce qui peut être récupéré par une fonction d'assistance ou une méthode de super-classe définissant getBalanceen termes de charge.

(Il s'agit d'un modèle de conception, pour lequel je ne me souviens pas d'un nom spécifique: définir une interface conviviale complexe en termes d'interface minimale conviviale pour les implémenteurs. Dans Classic Mac OS, l'interface minimale pour les opérations de dessin était appelée le "goulot d'étranglement" mais cela ne semble pas être un terme populaire.)

Si ce n'est pas le cas (il y a peu d'implémentations ou exactement une) charge(), il est logique de séparer les méthodes pour plus de clarté et pour permettre l'ajout simple de comportements pertinents pour des charges non nulles .

Kevin Reid
la source
1
En effet, j'ai eu des cas où l'interface la plus minimale a été choisie pour faciliter le test de couverture complète. Mais cela n'est pas nécessairement exposé à l'utilisateur de la classe. Par exemple, insérer , ajouter et supprimer peuvent tous être de simples enveloppes d'un remplacement général qui a tous les chemins de contrôle bien étudiés et testés.
JDługosz
J'ai vu une telle API. L'allocateur Lua 5.1+ l'utilise. Vous fournissez une fonction et, en fonction des paramètres qu'elle transmet, vous décidez si elle essaie d'allouer de la nouvelle mémoire, de désallouer de la mémoire ou de réallouer la mémoire de l'ancienne mémoire. Il est pénible d'écrire de telles fonctions. Je préférerais de beaucoup que Lua vous ait donné deux fonctions, une pour l'allocation et une pour la désallocation.
Nicol Bolas
@NicolBolas: Et aucun pour la réaffectation? Quoi qu'il en soit, celui-ci a l '«avantage» supplémentaire d'être presque le même que realloc, parfois, sur certains systèmes.
Deduplicator
@Deduplicator: allocation vs réallocation est ... OK. Ce n'est pas génial de les mettre dans la même fonction, mais ce n'est pas aussi mauvais que de mettre la désallocation dans la même. C'est aussi une distinction facile à faire.
Nicol Bolas
Je dirais que confondre la désallocation avec la (ré) allocation est un exemple particulièrement mauvais de ce type d'interface. (La désallocation a un contrat très différent: il n'aboutit pas à un pointeur valide.)
Kevin Reid