La priorité des méthodes concrètes est-elle une odeur de code?

31

Est-il vrai que remplacer des méthodes concrètes est une odeur de code? Parce que je pense que si vous avez besoin de remplacer des méthodes concrètes:

public class A{
    public void a(){
    }
}

public class B extends A{
    @Override
    public void a(){
    }
}

il peut être réécrit comme

public interface A{
    public void a();
}

public class ConcreteA implements A{
    public void a();
}

public class B implements A{
    public void a(){
    }
}

et si B veut réutiliser a () dans A, il peut être réécrit comme:

public class B implements A{
    public ConcreteA concreteA;
    public void a(){
        concreteA.a();
    }
}

qui ne nécessite pas d'héritage pour remplacer la méthode, est-ce vrai?

ggrr
la source
4
Voté parce que (dans les commentaires ci-dessous), Telastyn et Doc Brown s'accordent sur la priorité, mais donnent des réponses opposées oui / non à la question principale parce qu'ils ne sont pas d'accord sur la signification de "odeur de code". Ainsi, une question destinée à être sur la conception de code a été fortement couplée à un morceau d'argot. Et je le pense inutilement, car la question porte spécifiquement sur une technique contre une autre.
Steve Jessop
5
La question n'est pas balisée Java, mais le code semble être Java. En C # (ou C ++), vous devez utiliser le virtualmot - clé pour prendre en charge les remplacements. Ainsi, la question est rendue plus (ou moins) ambiguë par les particularités de la langue.
Erik Eidt
1
Il semble que presque toutes les fonctionnalités du langage de programmation finissent inévitablement par être classées comme une "odeur de code" après suffisamment d'années.
Brandon
2
J'ai l'impression que le finalmodificateur existe exactement pour des situations comme celles-ci. Si le comportement de la méthode est tel qu'elle n'est pas conçue pour être remplacée, marquez-la comme final. Sinon, attendez-vous à ce que les développeurs choisissent de le remplacer.
Martin Tuskevicius

Réponses:

34

Non, ce n'est pas une odeur de code.

  • Si une classe n'est pas définitive, elle permet d'être sous-classée.
  • Si une méthode n'est pas définitive, elle permet d'être remplacée.

Il incombe à chaque classe d'examiner attentivement si le sous-classement est approprié et quelles méthodes peuvent être remplacées .

La classe peut se définir elle-même ou n'importe quelle méthode comme finale, ou peut imposer des restrictions (modificateurs de visibilité, constructeurs disponibles) sur la manière et l'endroit où elle est sous-classée.

Le cas habituel pour remplacer les méthodes est une implémentation par défaut dans la classe de base qui peut être personnalisée ou optimisée dans la sous-classe (en particulier dans Java 8 avec l'avènement des méthodes par défaut dans les interfaces).

class A {
    public String getDescription(Element e) {
        // return default description for element
    }
}

class B extends A {
    public String getDescription(Element e) {
        // return customized description for element
    }
}

Une alternative pour remplacer le comportement est le modèle de stratégie , où le comportement est résumé sous forme d'interface, et l'implémentation peut être définie dans la classe.

interface DescriptionProvider {
    String getDescription(Element e);
}

class A {
    private DescriptionProvider provider=new DefaultDescriptionProvider();

    public final String getDescription(Element e) {
       return provider.getDescription(e);
    }

    public final void setDescriptionProvider(@NotNull DescriptionProvider provider) {
        this.provider=provider;
    }
}

class B extends A {
    public B() {
        setDescriptionProvider(new CustomDescriptionProvider());
    }
}
Peter Walser
la source
Je comprends que, dans le monde réel, remplacer une méthode concrète peut être une odeur de code pour certains. Mais, j'aime cette réponse car elle me semble plus basée sur les spécifications.
Darkwater23
Dans votre dernier exemple, ne devrait pas Aimplémenter DescriptionProvider?
Repéré le
@Spotted: A pourrait implémenter DescriptionProvider, et donc être le fournisseur de description par défaut. Mais l'idée ici est la séparation des préoccupations: a Abesoin de la description (d'autres classes pourraient aussi), tandis que d'autres implémentations les fournissent.
Peter Walser
2
Ok, ça ressemble plus au modèle de stratégie .
Repéré le
@Spotted: vous avez raison, l'exemple illustre le modèle de stratégie, pas le modèle de décorateur (un décorateur ajouterait un comportement à un composant). Je vais corriger ma réponse, merci.
Peter Walser
25

Est-il vrai que remplacer des méthodes concrètes est une odeur de code?

Oui, en général, remplacer les méthodes concrètes est une odeur de code.

Parce que la méthode de base a un comportement associé que les développeurs respectent généralement, la modification qui entraînera des bogues lorsque votre implémentation fera quelque chose de différent. Pire, s'ils modifient le comportement, votre implémentation précédemment correcte peut exacerber le problème. Et en général, il est assez difficile de respecter le principe de substitution de Liskov pour les méthodes concrètes non triviales.

Maintenant, il y a des cas où cela peut être fait et c'est bien. Les méthodes semi-triviales peuvent être remplacées en toute sécurité. Les méthodes de base qui n'existent qu'en tant que type de comportement "par défaut sain" destiné à être remplacé ont de bonnes utilisations.

Par conséquent, cela me semble être une odeur - parfois bonne, généralement mauvaise, jetez un coup d'œil pour vous en assurer.

Telastyn
la source
29
Votre explication est bonne, mais j'arrive exactement à la conclusion opposée: "non, remplacer des méthodes concrètes n'est pas une odeur de code en général" - c'est seulement une odeur de code quand on remplace des méthodes qui n'étaient pas destinées à être surchargées. ;-)
Doc Brown
2
@DocBrown Exactement! Je vois également que l'explication (et en particulier la conclusion) s'oppose à la déclaration initiale.
Tersosauros
1
@Spotted: le contre-exemple le plus simple est une Numberclasse avec une increment()méthode qui définit la valeur à sa prochaine valeur valide. Si vous le sous-classe dans EvenNumberincrement()ajoute deux au lieu d'un (et le setter applique la régularité), la première proposition de l'OP nécessite des implémentations en double de chaque méthode de la classe et sa seconde nécessite l'écriture de méthodes wrapper pour appeler les méthodes dans le membre. L'héritage vise en partie à ce que les classes d'enfants expliquent clairement en quoi elles diffèrent des parents en ne fournissant que les méthodes qui doivent être différentes.
Blrfl
5
@DocBrown: être une odeur de code n'implique pas que quelque chose soit nécessairement mauvais, seulement que c'est probable .
mikołak
3
@docbrown - pour moi, cela coïncide avec "privilégier la composition à l'héritage". Si vous préférez les méthodes concrètes, vous êtes déjà dans une terre à odeur minuscule pour avoir un héritage. Quelles sont les chances que vous surpassiez quelque chose que vous ne devriez pas être? D'après mon expérience, je pense que c'est probable. YMMV.
Telastyn
7

si vous avez besoin de remplacer des méthodes [...] concrètes, il peut être réécrit comme

S'il peut être réécrit de cette façon, cela signifie que vous avez le contrôle sur les deux classes. Mais alors vous savez si vous avez conçu la classe A pour être dérivée de cette façon et si vous l'avez fait, ce n'est pas une odeur de code.

Si, d'autre part, vous n'avez pas de contrôle sur la classe de base, vous ne pouvez pas réécrire de cette façon. Donc, soit la classe a été conçue pour être dérivée de cette façon, dans ce cas, allez-y, ce n'est pas une odeur de code, ou ce n'était pas le cas, dans ce cas, vous avez deux options: chercher une autre solution tout à fait, ou continuer quand même , car le travail l'emporte sur la pureté de la conception.

Jan Hudec
la source
Si la classe A aurait été conçue pour être dérivée, ne serait-il pas plus logique d'avoir une méthode abstraite pour exprimer clairement l'intention? Comment celui qui outrepasse la méthode peut-il savoir que la méthode a été conçue de cette façon?
Repéré le
@Spotted, il existe de nombreux cas où des implémentations par défaut peuvent être fournies et chaque spécialisation n'a besoin que de remplacer certaines d'entre elles, soit pour étendre les fonctionnalités, soit pour être efficace. L'exemple extrême est les visiteurs. L'utilisateur sait comment les méthodes ont été conçues à partir de la documentation; il doit être là pour expliquer ce que l'on attend de la dérogation de toute façon.
Jan Hudec
Je comprends votre point de vue, je pense toujours qu'il est dangereux que le seul moyen pour un développeur, afin de savoir qu'une méthode peut être annulée, est de lire le document parce que: 1) si elle doit être annulée, je n'ai aucune garantie que ce soit sera fait. 2) si elle ne doit pas être annulée, quelqu'un le fera pour sa commodité 3) tout le monde ne lit pas le document. De plus, je n'ai jamais été confronté à un cas où je devais remplacer une méthode entière, mais uniquement des parties (et j'ai fini par utiliser un modèle de modèle).
Repéré le
@Spotted, 1) si elle doit être remplacée, elle devrait l'être abstract; mais il y a de nombreux cas où cela n'a pas à être. 2) si elle ne doit pas être remplacée, elle doit l'être final(non virtuelle). Mais il existe encore de nombreux cas où les méthodes peuvent être remplacées. 3) si le développeur en aval ne lit pas les documents, ils vont se tirer une balle dans le pied, mais ils le feront, quelles que soient les mesures que vous mettez en place.
Jan Hudec
Ok donc la divergence de notre point de vue ne réside que dans 1 mot. Vous dites que chaque méthode doit être abstraite ou définitive tandis que je dis que chaque méthode doit être abstraite ou définitive. :) Quels sont ces cas où le remplacement d'une méthode est nécessaire (et sûr)?
Repéré le
3

Tout d'abord, permettez-moi de souligner que cette question ne s'applique qu'à certains systèmes de dactylographie. Par exemple, en tapant structurel et frappe de canard systèmes - une classe tout simplement avoir une méthode avec le même nom et la signature ne signifie que la classe était de type compatible (au moins pour cette méthode particulière, dans le cas de la saisie de canard).

Ceci est (évidemment) très différent du domaine des langages typés statiquement , tels que Java, C ++, etc. Ainsi que la nature du typage fort , tel qu'utilisé à la fois en Java et C ++, ainsi qu'en C # et autres.


Je suppose que vous venez d'un arrière-plan Java, où les méthodes doivent être explicitement marquées comme finales si le concepteur de contrat souhaite qu'elles ne soient pas remplacées. Cependant, dans des langages tels que C #, l'inverse est la valeur par défaut. Les méthodes sont (sans décoration, mots-clés spéciaux) " scellées " (version C # de final), et doivent être explicitement déclarées comme virtualsi elles pouvaient être remplacées.

Si une API ou une bibliothèque a été conçue dans ces langages avec une méthode concrète qui peut être remplacée, alors elle doit (au moins dans certains cas) avoir un sens pour qu'elle soit remplacée. Par conséquent, ce n'est pas une odeur de code de remplacer une finalméthode non scellée / virtuelle / non concrète.     (Cela suppose bien sûr que les concepteurs de l'API suivent les conventions et marquent en tant que virtual(C #) ou marquent en tant que final(Java) les méthodes appropriées pour ce qu'ils conçoivent.)


Voir également

Tersosauros
la source
Dans le typage canard, deux classes ayant la même signature de méthode sont-elles suffisantes pour être compatibles avec le type, ou est-il également nécessaire que les méthodes aient la même sémantique, de sorte qu'elles soient liskov substituables à une classe de base implicite?
Wayne Conrad
2

Oui c'est parce que ça peut conduire à appeler super anti-pattern. Il peut également dénaturer l'objectif initial de la méthode, introduire des effets secondaires (=> bugs) et faire échouer les tests. Utiliseriez-vous une classe dont les tests unitaires sont rouges (failling)? Je ne le ferai pas.

J'irai encore plus loin en disant que l'odeur de code initiale est quand une méthode n'est pas abstractni final. Parce qu'il permet de modifier (en redéfinissant) le comportement d'une entité construite (ce comportement peut être perdu ou altéré). Avec abstractet finall'intention est sans ambiguïté:

  • Résumé: vous devez fournir un comportement
  • Final: vous n'êtes pas autorisé à modifier le comportement

La façon la plus sûre d'ajouter un comportement à une classe existante est ce que montre votre dernier exemple: utiliser le motif décorateur.

public interface A{
    void a();
}

public final class ConcreteA implements A{
    public void a() {
        ...
    }
}

public final class B implements A{
    private final ConcreteA concreteA;
    public void a(){
        //Additionnal behaviour
        concreteA.a();
    }
}
À pois
la source
9
Cette approche en noir et blanc n'est pas vraiment très utile. Pensez Object.toString()à Java ... Si c'était le cas abstract, de nombreuses choses par défaut ne fonctionneraient pas, et le code ne serait même pas compilé lorsque quelqu'un n'a pas outrepassé la toString()méthode! Si c'était le cas final, les choses seraient encore pires - aucune possibilité d'autoriser la conversion d'objets en chaîne, à l'exception de la méthode Java. Comme le dit la réponse de @Peter Walser , les implémentations par défaut (telles que Object.toString()) sont importantes. Surtout dans les méthodes par défaut de Java 8.
Tersosauros
@Tersosauros Vous avez raison, si toString()c'était le cas abstractou finalce serait une douleur. Mon opinion personnelle est que cette méthode est cassée et il aurait été préférable de ne pas l'avoir du tout (comme equals et hashCode ). Le but initial des valeurs par défaut Java est de permettre l'évolution de l'API avec la rétrocompatibilité.
Repéré le
Le mot «rétrocompatibilité» a énormément de syllabes.
Craig
@Spotted Je suis très déçu de l'acceptation générale de l'héritage concret sur ce site, je m'attendais à mieux: / Votre réponse devrait avoir beaucoup plus de votes positifs
TheCatWhisperer
1
@TheCatWhisperer Je suis d'accord, mais d'un autre côté, il m'a fallu si longtemps pour découvrir les avantages subtils de l'utilisation de la décoration par rapport à l'héritage concret (en fait, il est devenu clair lorsque j'ai commencé à écrire des tests en premier) que vous ne pouvez blâmer personne pour cela . La meilleure chose à faire est d'essayer d'éduquer les autres jusqu'à ce qu'ils découvrent la vérité (blague). :)
Repéré le