Le code comme celui-ci est-il une «épave de train» (en violation de la loi de Déméter)?

23

En parcourant le code que j'ai écrit, je suis tombé sur la construction suivante qui m'a fait réfléchir. À première vue, il semble assez propre. Oui, dans le code réel, la getLocation()méthode a un nom légèrement plus spécifique qui décrit mieux exactement l'emplacement qu'elle obtient.

service.setLocation(this.configuration.getLocation().toString());

Dans ce cas, serviceest une variable d'instance d'un type connu, déclarée dans la méthode. this.configurationvient d'être passé au constructeur de classe, et est une instance d'une classe implémentant une interface spécifique (qui rend obligatoire une getLocation()méthode publique ). Par conséquent, le type de retour de l'expression this.configuration.getLocation()est connu; spécifiquement dans ce cas, c'est un java.net.URL, alors que service.setLocation()veut un String. Étant donné que les deux types String et URL ne sont pas directement compatibles, une sorte de conversion est nécessaire pour adapter la cheville carrée dans le trou rond.

Toutefois , selon la loi de Déméter cité dans code propre , une méthode f dans la classe C ne devrait appeler des méthodes sur C , les objets créés par ou passés comme arguments à f et objets retenus en variables d'instance de C . Tout ce qui est au-delà (le dernier toString()dans mon cas particulier ci-dessus, à moins que vous ne considériez un objet temporaire créé à la suite de l'invocation de la méthode elle-même, auquel cas toute la loi semble être théorique) est interdit.

Existe-t-il un raisonnement valable pour lequel un appel comme celui-ci, étant donné les contraintes énumérées, devrait être découragé ou même refusé? Ou suis-je juste trop nerveux?

Si je devais implémenter une méthode URLToString()qui appelle simplement toString()un URLobjet (tel que celui renvoyé par getLocation()) qui lui est passé en tant que paramètre et retourne le résultat, je pourrais envelopper l' getLocation()appel pour obtenir exactement le même résultat; efficacement, je voudrais simplement déplacer la conversion d'un pas vers l'extérieur. Serait-ce en quelque sorte acceptable? (Il me semble , intuitivement, que cela ne devrait pas faire de différence de toute façon, car cela ne fait que déplacer un peu les choses. Cependant, selon la lettre de la loi de Déméter citée, ce serait acceptable, car je fonctionnerait alors directement sur un paramètre d'une fonction.)

Cela ferait-il une différence s'il s'agissait de quelque chose d'un peu plus exotique que de faire appel toString()à un type standard?

Lorsque vous répondez, gardez à l'esprit qu'il servicen'est pas pratique de modifier le comportement ou l'API du type de la variable. Aussi, pour les besoins de l'argument, disons que la modification du type de retour de getLocation()est également impossible.

un CVn
la source

Réponses:

34

Le problème ici est la signature de setLocation. Il est strictement tapé .

Pour élaborer: pourquoi s'attendrait-il String? A Stringreprésente tout type de données textuelles . Cela peut être tout sauf un emplacement valide.

En fait, cela pose une question: qu'est-ce qu'un emplacement? Comment puis- je savoir sans regarder dans votre code? Si c'était le cas, URLj'en saurais beaucoup plus sur ce que cette méthode attend.
Il serait peut-être plus logique que ce soit une classe personnalisée Location. D'accord, je ne saurais pas au début ce que c'est, mais à un moment donné (probablement avant d'écrire, this.configuration.getLocation()je prendrais une minute pour comprendre ce que cette méthode renvoie).
Certes, dans les deux cas, je dois chercher un autre endroit pour comprendre ce qui est attendu. Cependant, dans ce dernier cas, si je comprends bien ce qu'est un Location, je peux utiliser votre API, dans le premier cas, si je comprends ce qu'est un String(qui peut être attendu), je ne sais toujours pas ce que votre API attend.

Dans le scénario peu probable, où un emplacement est n'importe quel type de données textuelles, je réinterpréterais ceci à n'importe quel type de données, qui a une représentation textuelle . Étant donné le fait que cela Objecta une toStringméthode, vous pouvez aller avec cela, même si cela nécessite un acte de foi de la part des clients de votre code.

Vous devez également considérer que c'est de Java que vous parlez, qui a très peu de fonctionnalités par conception. C'est ce qui vous oblige à réellement appeler le toStringà la fin.
Si vous prenez C # par exemple, qui est également typé statiquement, vous pourrez en fait omettre cet appel en définissant le comportement d'une conversion implicite .
Dans les langages typés dynamiquement, comme Objective-C, vous n'avez pas vraiment besoin de la conversion non plus, car tant que la valeur se comporte comme une chaîne, tout le monde est content.

On pourrait soutenir que le dernier appel à toStringest moins un appel que le simple bruit généré par la demande d'explicitation de Java. Vous appelez une méthode, que tout objet Java possède, donc vous n'encodez en fait aucune connaissance sur une "unité distante" et ne violez donc pas le Principe de moindre connaissance. Il n'y a aucun moyen, peu importe ce qui getLocationrevient, qu'il n'a pas de toStringméthode.

Mais s'il vous plaît, n'utilisez pas de chaînes, à moins qu'elles ne soient vraiment le choix le plus naturel (ou à moins que vous n'utilisiez une langue, qui n'a même pas d'énumérations ... été là).

back2dos
la source
J'aurais tendance à être d'accord avec vous, mais il a déjà dit qu'il ne pouvait pas modifier l'API de service.
jhocking
1
@jhocking: Il n'a pas dit qu'il ne pouvait pas. Il a dit que ce n'était pas pratique. Je ne suis pas d'accord. Essayer d'appliquer les meilleures pratiques au code qui contourne les défauts de conception d'API n'a vraiment de sens que si de telles solutions de contournement sont la seule option possible. Cependant, ici, définir l'API directement est la meilleure option. Il est toujours préférable de supprimer les carences que de les contourner.
back2dos
bien +1 de toute façon pour "est moins un appel que juste du bruit généré par la demande d'explicitation de Java"
jhocking
1
Je donnerais ce +1 même si ce n'est que pour "typé". Dans mon code, j'essaie de transmettre autant que possible les types qui expriment le sens / l'intention, mais lorsque vous travaillez avec des bibliothèques et des API tierces, vous êtes parfois un peu coincé à utiliser ce que les auteurs de cette API ont décidé. Et IIRC, un emplacement peut en effet être "n'importe quel type de données textuelles", mais pour l'implémentation sur laquelle je travaille, un emplacement non-URL n'a aucun sens.
un CVn du
1
Quant à changer l'API; il s'agit d'un logiciel informatique, il est donc pratiquement toujours possible de changer les choses. (Si rien d'autre, vous pouvez toujours écrire une couche d'abstraction.) Cependant, cela implique parfois beaucoup trop d'efforts à court et à long terme pour être justifié. Ensuite, il pourrait être parfaitement possible d'apporter de tels changements, mais toujours peu pratique.
un CVn du
21

La loi de Demeter est une directive de conception, pas une loi à suivre religieusement.

Si vous pensez que vos classes sont suffisamment découplées, il n'y a rien de mal à la ligne, this.configuration.getLocation()surtout si, comme vous le dites, il n'est pas pratique de changer d'autres parties de l'API.

Je suis à peu près sûr que le client sera parfaitement heureux même si vous faites la loi de Demeter en pièces, tant que vous livrez ce qu'il veut et à temps. Ce qui n'est pas une excuse pour fabriquer de mauvais logiciels, mais un rappel pour être pragmatique lors du développement de logiciels.

Trasplazio Garzuglio
la source
1
Étant donné qu'il this.configurations'agit d'une variable d'instance d'un type d'interface connue, l'appel d'une méthode définie par cette interface semble correct, même selon une interprétation stricte. Oui, je sais que c'est une ligne directrice, tout comme KISS, SOLID, YAGNI et ainsi de suite. Il y a très peu ou pas de «lois» (au sens juridique) dans le développement général de logiciels.
un CVn le
4
+1 pour être pragmatique ;-)
Treb
1
Je ne pense pas que la loi de Dementer devrait même s'appliquer dans un cas comme celui-ci - la configuration est fondamentalement un conteneur. Dans chaque API, j'ai déjà travaillé avec la recherche à l'intérieur des conteneurs est un comportement normal et attendu.
Loren Pechtel
2

La seule chose à laquelle je peux penser de ne pas écrire de code comme celui-ci est que si this.configuration.getLocation()renvoie null? Cela dépend de votre code environnant et du public cible utilisant ce code. Mais comme le dit Marco - la loi de Demeter est une règle d'or - c'est bon à suivre, mais ne vous cassez pas le dos en le faisant inutilement.

Martyn
la source
Si this.configuration.getLocation()retournait null, alors soit nous (a) n'aurions probablement jamais été aussi loin, soit (b) quelque chose de catastrophique s'est produit entre-temps, auquel cas je voudrais que le code échoue. Donc, bien que ce soit certainement un point valable en général, dans ce cas particulier, il est assez sûr de dire qu'il ne s'applique pas. En outre, tout cela et bien plus encore est entouré d'un gestionnaire d'exceptions spécialement conçu pour faire face à ce type d'échecs inattendus.
un CVn le
2

Suivre strictement la loi de Demeter signifierait que vous devriez implémenter une méthode dans l'objet de configuration comme:

function getLocationAsString() {
  return getLocation().toString();
}

mais personnellement, cela ne me dérangerait pas parce que c'est une si petite situation, et de toute façon vos mains sont un peu liées à cause de l'API que vous ne pouvez pas changer. Les règles de programmation concernent ce que vous devez faire lorsque vous avez le choix, mais parfois vous n'avez pas le choix.

jhocking
la source
1
C'est la même suggestion ici: c2.com/cgi/wiki?TrainWreck "Créez une méthode qui représente le comportement souhaité et indique au client quoi faire. Cela suit le principe" dites, ne demandez pas "."
heltonbiker