Pourquoi Clean Code suggère-t-il d'éviter les variables protégées?

168

Clean Code suggère d'éviter les variables protégées dans la section "Distance verticale" du chapitre "Mise en forme":

Les concepts étroitement liés doivent rester proches les uns des autres, à la verticale. Il est clair que cette règle ne fonctionne pas pour les concepts qui appartiennent à des fichiers séparés. Mais les concepts étroitement liés ne doivent pas être séparés dans des fichiers différents, à moins que vous n’ayez une très bonne raison. En effet, c'est l' une des raisons pour lesquelles les variables protégées doivent être évitées .

Quel est le raisonnement?

Matsemann
la source
7
montrer un exemple où vous pensez que vous avez besoin , il
moucheron
63
Je ne prendrais pas grand chose dans ce livre comme évangile.
Rig
40
@Rig, vous bloquez le blasphème dans ces régions avec un commentaire comme celui-là.
Ryathal
40
Je suis d'accord avec ça. J'ai lu le livre et peux avoir ma propre opinion à ce sujet.
Rig
10
J'ai lu le livre et bien que je sois d'accord avec l' essentiel de ce qu'il contient , je ne dirais pas que je le considère comme un évangile non plus. Je pense que chaque situation est différente… et être capable de s'en tenir aux directives exactes décrites dans le livre est assez difficile pour beaucoup de projets.
Simon Whitehead

Réponses:

277

Les variables protégées doivent être évitées pour les raisons suivantes:

  1. Ils ont tendance à conduire à des problèmes YAGNI . À moins que vous n'ayez une classe descendante qui traite réellement avec le membre protégé, rendez-la privée.
  2. Ils ont tendance à conduire à des problèmes de LSP . Les variables protégées sont généralement associées à une invariance intrinsèque (sinon elles seraient publiques). Les héritiers doivent alors conserver ces propriétés, que les gens peuvent bousiller ou violer volontairement.
  3. Ils ont tendance à violer OCP . Si la classe de base émet trop d'hypothèses sur le membre protégé ou si l'héritier est trop souple avec le comportement de la classe, le comportement de la classe de base peut être modifié par cette extension.
  4. Ils ont tendance à conduire à l'héritage par extension plutôt que par composition. Cela tend à conduire à un couplage plus étroit, à davantage de violations de la SRP , à des tests plus difficiles et à une foule d'autres choses qui relèvent de la discussion sur la "composition préférentielle sur l'héritage".

Mais comme vous le voyez, tout cela est "tendance à". Parfois, un membre protégé est la solution la plus élégante. Et les fonctions protégées ont tendance à avoir moins de ces problèmes. Mais il y a un certain nombre de choses qui les font traiter avec précaution. Avec tout ce qui nécessite ce genre de soin, les gens vont faire des erreurs et dans le monde de la programmation, cela signifie des bugs et des problèmes de conception.

Telastyn
la source
Merci pour beaucoup de bonnes raisons. Cependant, le livre le mentionne spécifiquement dans un contexte de formatage et de distance verticale, c’est la partie sur laquelle je me pose des questions.
Matsemann
2
Il semblerait qu'Oncle Bob parle de distance verticale lorsqu'une personne fait référence à un membre protégé entre des classes et non dans la même classe.
Robert Harvey
5
@Matsemann - bien sûr. Si je me souviens bien du livre, cette section se concentre sur la lisibilité et la capacité de découverte du code. Si les variables et les fonctions fonctionnent sur un concept, elles doivent être proches dans le code. Les membres protégés seront utilisés par des types dérivés, qui ne peuvent pas nécessairement être proches du membre protégé car ils se trouvent dans un fichier complètement différent.
Telastyn
2
@ steve314 Je ne peux pas imaginer un scénario dans lequel la classe de base et tous ses héritiers ne vivront jamais que dans un seul fichier. Sans exemple, je suis enclin à penser que c'est un abus d'héritage ou une piètre abstraction.
Telastyn
3
YAGNI = Vous n'en aurez pas besoin LSP = Principe de substitution de Liskov OCP = Principe d'ouverture / fermeture SRP = Principe de responsabilité unique
Bryan Glazer
54

C'est vraiment la même raison pour laquelle vous évitez les globaux, mais à plus petite échelle. C'est parce qu'il est difficile de trouver partout une variable lue, ou pire, d'écrire, et difficile de rendre l'utilisation cohérente. Si l'utilisation est limitée, par exemple si vous écrivez à un endroit évident de la classe dérivée et lisez à un endroit évident de la classe de base, les variables protégées peuvent alors rendre le code plus clair et concis. Si vous êtes tenté de lire et d'écrire une variable à volonté, dans plusieurs fichiers, il est préférable de l'encapsuler.

Karl Bielefeldt
la source
26

Je n'ai pas lu le livre, mais je peux comprendre ce que voulait dire Oncle Bob.

Si vous mettez protectedquelque chose, cela signifie qu'une classe peut en hériter. Mais les variables membres sont supposées appartenir à la classe dans laquelle elles sont contenues; cela fait partie de l'encapsulation de base. Le fait d'insérer protectedune variable membre interrompt l'encapsulation, car une classe dérivée a désormais accès aux détails d'implémentation de la classe de base. C'est le même problème qui se produit lorsque vous créez une variable publicsur une classe ordinaire.

Pour corriger le problème, vous pouvez encapsuler la variable dans une propriété protégée, comme suit:

protected string Name
{
    get { return name; }
    private set { name = value; }
}

Cela permet named’être défini en toute sécurité à partir d’une classe dérivée à l’aide d’un argument de constructeur, sans exposer les détails d’implémentation de la classe de base.

Robert Harvey
la source
Vous avez créé une propriété publique avec un setter protégé. Le champ protégé doit être résolu avec une propriété protégée avec un passeur privé.
Andy
2
+1 maintenant. Setter pourrait aussi être protégé, mais les identifiants de point que la base peut appliquer si la nouvelle valeur est valide ou non.
Andy
Juste pour offrir un point de vue opposé - toutes mes variables sont protégées dans une classe de base. Si elles ne doivent être accessibles que via une interface publique, il ne devrait pas s'agir d'une classe dérivée, elle devrait simplement contenir un objet baseclass. Mais j'évite aussi les hiérarchies de plus de 2 classes de profondeur
Martin Beckett
2
Il y a une énorme différence entre protectedet publicmembres. Si BaseTypeexpose un membre public, cela implique que tous les types dérivés doivent faire fonctionner le même membre de la même manière, puisqu’une DerivedTypeinstance peut être transmise au code qui reçoit une BaseTyperéférence et compte utiliser ce membre. En revanche, si BaseTypeexpose un protectedmembre, alors DerivedTypepeut espérer accéder à ce membre dans base, mais basene peut être autre chose qu'un BaseType.
Supercat
18

L'argument d'Oncle Bob est principalement basé sur la distance: si vous avez un concept important pour une classe, associez le concept à la classe dans ce fichier. Non séparé sur deux fichiers sur le disque.

Les variables des membres protégés sont dispersées à deux endroits et ressemblent à de la magie. Vous faites référence à cette variable, pourtant elle n'est pas définie ici ... où est- elle définie? Et ainsi commence la chasse. Mieux vaut éviter protectedcomplètement son argument.

Maintenant, je ne crois pas que cette règle doive être obéie à la lettre tout le temps (comme: tu ne dois pas utiliser protected). Observez l’ esprit de ce qu’il cherche à faire: regrouper les éléments liés dans un seul fichier - utilisez des techniques et des fonctionnalités de programmation pour le faire. Je vous recommande de ne pas trop analyser et de ne pas vous perdre dans les détails à ce sujet.

J. Polfer
la source
9

Quelques très bonnes réponses déjà ici. Mais une chose à ajouter:

Dans la plupart des langages OO modernes, c'est une bonne habitude (en Java, autant que cela soit nécessaire) de placer chaque classe dans son propre fichier (ne vous inquiétez pas du C ++ où vous avez souvent deux fichiers).

Il est donc pratiquement impossible de conserver les fonctions dans une classe dérivée en accédant à une variable protégée d'une classe de base "verticalement proche" dans le code source de la définition de la variable. Mais idéalement, elles devraient y être conservées car les variables protégées sont destinées à un usage interne, ce qui fait que les fonctions qui les accèdent sont souvent "un concept étroitement lié".

Doc Brown
la source
1
Pas à Swift. Il est intéressant de noter que Swift n’a pas «protégé», mais «privé» qui remplace à la fois «protégé» et «ami» C ++.
gnasher729
@ gnasher729 - bien sûr, Swift a beaucoup de Smalltalk dans son héritage. Smalltalk n'a rien d'autre que des variables privées et des méthodes publiques. Et privé en Smalltalk signifie privé: deux objets, même de la même classe, ne peuvent pas accéder aux variables de l'autre.
Jules
4

L'idée de base est qu'un "champ" (variable de niveau instance) déclaré comme protégé est probablement plus visible qu'il ne doit l'être et moins "protégé" que vous ne le souhaiteriez peut-être. Il n'y a pas de modificateur d'accès en C / C ++ / Java / C # équivalent à "accessible uniquement par les classes enfants dans le même assemblage", ce qui vous permet de définir vos propres enfants pouvant accéder au champ de votre assemblage. ne pas autoriser le même accès pour les enfants créés dans d'autres assemblées; C # a des modificateurs internes et protégés, mais leur combinaison rend l'accès "interne ou protégé", pas "interne et protégé". Ainsi, un champ protégé est accessible à tout enfant, que vous l'ayez écrit ou que quelqu'un d'autre l'ait fait. Protected est donc une porte ouverte à un pirate informatique.

De plus, les champs, de par leur définition, n’ont pratiquement pas de validation inhérente à leur modification. en C #, vous pouvez en créer un en lecture seule, ce qui rend les types de valeur effectivement constants et les types de référence ne pouvant pas être réinitialisés (mais toujours très mutables), mais c'est à peu près tout. En tant que tels, même protégés, vos enfants (en qui vous ne pouvez pas avoir confiance) ont accès à ce champ et peuvent lui attribuer une valeur non valide, ce qui rend l'état de l'objet incohérent (à éviter).

La méthode acceptée pour travailler avec les champs consiste à les rendre privés et à y accéder avec une propriété et / ou une méthode d'accesseur en lecture et en écriture. Si tous les consommateurs de la classe ont besoin de la valeur, rendez le getter (au moins) public. Si seuls les enfants en ont besoin, protégez le getter.

Une autre approche qui répond à la question est de vous demander; pourquoi le code dans une méthode enfant nécessite-t-il la possibilité de modifier directement mes données d'état? Qu'est-ce que cela dit à propos de ce code? C'est l'argument de "distance verticale" à sa face. S'il y a du code dans un enfant qui doit modifier directement l'état parent, alors peut-être que ce code devrait appartenir au parent en premier lieu?

KeithS
la source
4

C'est une discussion intéressante.

Pour être franc, de bons outils peuvent atténuer nombre de ces problèmes "verticaux". En fait, à mon avis, la notion de "fichier" est en réalité quelque chose qui gêne le développement logiciel - quelque chose sur lequel travaille le projet Light Table (http://www.chris-granger.com/2012/04/12/light- table --- a-new-ide-concept /).

Supprimez les fichiers de la photo, et la portée protégée devient beaucoup plus attrayante. Le concept protégé devient plus clair dans des langages comme SmallTalk - où tout héritage étend simplement le concept original d'une classe. Il devrait tout faire et avoir tout ce que la classe parente a fait.

À mon avis, les hiérarchies de classes devraient être aussi peu profondes que possible, la plupart des extensions provenant de la composition. Dans de tels modèles, je ne vois aucune raison pour que les variables privées ne soient pas protégées à la place. Si la classe étendue doit représenter une extension incluant tout le comportement de la classe parente (ce qui, à mon avis, devrait l'être et donc que les méthodes privées sont intrinsèquement mauvaises), pourquoi la classe étendue ne représenterait-elle pas également le stockage de ces données?

En d'autres termes - dans tous les cas où, à mon avis, une variable privée conviendrait mieux à une variable protégée, l'héritage n'est pas la solution au problème à l'origine.

Spronkey
la source
0

Comme il est dit ici, ce n’est qu’une des raisons, c’est-à-dire que le code lié logiquement doit être placé dans des entités physiques liées (fichiers, packages et autres). Sur une plus petite échelle, ceci est identique à la raison pour laquelle vous ne mettez pas une classe qui effectue des requêtes de base de données à l'intérieur du package avec des classes qui affichent l'interface utilisateur.

Cependant, la principale raison pour laquelle les variables protégées ne sont pas recommandées est qu'elles tendent à rompre l'encapsulation. Les variables et les méthodes doivent avoir une visibilité aussi limitée que possible. Pour plus de détails, voir Effective Java de Joshua Bloch , article 13 - "Minimiser l’accessibilité des classes et des membres".

Cependant, vous ne devriez pas prendre tout cela à la lettre, mais simplement comme une guildline; Si les variables protégées sont très mauvaises, elles n'auraient pas été placées dans la langue. Un endroit raisonnable pour les champs protégés à l’intérieur est la classe de base à partir d’un framework de test (les classes de test d’intégration l’étendent).

m3th0dman
la source
1
Ne présumez pas qu’une langue le permet, c’est acceptable. Je ne suis pas sûr qu'il y ait vraiment une bonne raison d'autoriser les champs protégés; nous les refusons en fait chez mon employeur.
Andy
2
@Andy Vous n'avez pas lu ce que j'ai dit; J'ai dit que les champs protégés ne sont pas recommandés et les raisons. Il existe clairement des scénarios dans lesquels des variables protégées sont nécessaires; vous souhaiterez peut-être une valeur finale constante uniquement dans les sous-classes.
m3th0dman
Vous voudrez peut-être reformuler une partie de votre message, car vous semblez utiliser variable et champ de manière interchangeable et expliciter que vous considéreriez des choses comme des consts protégées comme une exception.
Andy
3
@Andy Il est assez évident que depuis que je parlais de variables protégées, je ne parlais pas de variables locales ou de paramètres, mais de champs. J'ai également mentionné un scénario dans lequel les variables protégées non constantes sont utiles; à mon humble avis, il est faux pour une entreprise de faire respecter la manière dont les programmeurs écrivent du code.
m3th0dman
1
Si c'était évident, je n'aurais pas été dérouté et je ne vous ai pas voté. La règle a été définie par nos développeurs expérimentés, de même que notre décision d'exécuter StyleCop, FxCop et d'exiger des tests unitaires et des révisions de code.
Andy
0

Je trouve que l’utilisation de variables protégées pour les tests JUnit peut être utile. Si vous les rendez privés, vous ne pourrez pas y accéder pendant les tests sans réfléchir. Cela peut être utile si vous testez un processus complexe avec de nombreux changements d'état.

Vous pouvez les rendre privées, avec des méthodes getter protégées, mais si les variables ne seront utilisées qu'en externe lors d'un test JUnit, je ne suis pas sûr que ce soit une meilleure solution.

pseudo
la source
-1

Oui, je conviens que c'est un peu étrange étant donné que (si je me souviens bien), le livre traite également de toutes les variables non privées comme des éléments à éviter.

Je pense que le problème avec le modificateur protected est que non seulement le membre est exposé à des sous-classes, il est également visible par l'ensemble du package. Je pense que c'est ce double aspect qu'Oncle Bob fait exception à la règle. Bien sûr, on ne peut pas toujours éviter les méthodes protégées, et donc la qualification de variable protégée.

Je pense qu’une approche plus intéressante consiste à rendre tout public, ce qui vous obligera à avoir de petites classes, ce qui rendra l’ensemble de la métrique de distance verticale plutôt discutable.

CurtainDog
la source