Nettoyer le code et les objets hybrides et l'envie de fonctionnalités

J'ai donc récemment fait quelques remaniements majeurs de mon code. L'une des principales choses que j'ai essayé de faire était de diviser mes classes en objets de données et objets de travail. Cela a été inspiré, entre autres, par cette section de Clean Code :

Hybrides

Cette confusion conduit parfois à des structures de données hybrides malheureuses qui sont à moitié objet et à moitié structure de données. Ils ont des fonctions qui font des choses importantes, et ils ont aussi des variables publiques ou des accesseurs et mutateurs publics qui, à toutes fins utiles, rendent les variables privées publiques, tentant d'autres fonctions externes d'utiliser ces variables comme un programme procédural utiliserait un Structure de données.

De tels hybrides rendent difficile l'ajout de nouvelles fonctions mais rendent également difficile l'ajout de nouvelles structures de données. Ce sont les pires des deux mondes. Évitez de les créer. Ils indiquent une conception confuse dont les auteurs ne savent pas - ou pire, ignorent - s'ils ont besoin d'une protection contre les fonctions ou les types.

Récemment, je regardais le code d'un de mes objets de travail (qui implémente le modèle de visiteur ) et j'ai vu ceci:

@Override
public void visit(MarketTrade trade) {
    this.data.handleTrade(trade);
    updateRun(trade);
}

private void updateRun(MarketTrade newTrade) {
    if(this.data.getLastAggressor() != newTrade.getAggressor()) {
        this.data.setRunLength(0);
        this.data.setLastAggressor(newTrade.getAggressor());
    }
    this.data.setRunLength(this.data.getRunLength() + newTrade.getLots());
}

Je me suis tout de suite dit "envie de fonctionnalité! Cette logique doit être dans la Dataclasse - spécifiquement dans la handleTrademéthode. handleTradeEt updateRundoit toujours se passer ensemble". Mais alors j'ai pensé que "la classe de données n'est qu'une publicstructure de données, si je commence à le faire, alors ce sera un objet hybride!"

Quoi de mieux et pourquoi? Comment décidez-vous quoi faire?

Le texte que vous avez cité contient de bons conseils, même si je remplacerai «structures de données» par «enregistrements», en supposant que quelque chose comme des structures est voulu. Les enregistrements ne sont que des agrégations stupides de données. Bien qu'ils puissent être mutables (et donc avec état dans un état d'esprit de programmation fonctionnelle), ils n'ont pas d'état interne, pas d'invariants à protéger. Il est tout à fait valide d'ajouter des opérations à un enregistrement pour en faciliter l'utilisation.

Par exemple, nous pouvons affirmer qu'un vecteur 3D est un enregistrement stupide. Cependant, cela ne devrait pas nous empêcher d'ajouter une méthode comme add, ce qui facilite l'ajout de vecteurs. L'ajout d'un comportement ne transforme pas un enregistrement (pas si stupide) en un hybride.

Cette ligne est franchie lorsque l'interface publique d'un objet nous permet de rompre l'encapsulation: il existe des éléments internes auxquels nous pouvons accéder directement, amenant ainsi l'objet dans un état invalide. Il me semble que Datacela a un état, et qu'il peut être mis dans un état invalide:

• Après avoir traité un échange, le dernier agresseur peut ne pas être mis à jour.
• Le dernier agresseur peut être mis à jour même en l'absence de nouvelle transaction.
• La longueur d'exécution peut conserver son ancienne valeur même lorsque l'agresseur a été mis à jour.
• etc.

Si un état est valide pour vos données, alors tout va bien avec votre code, et vous pouvez continuer. Sinon: la Dataclasse est responsable de sa propre cohérence des données. Si la gestion d'un échange implique toujours la mise à jour de l'agresseur, ce comportement doit faire partie de la Dataclasse. Si le changement de l'agresseur implique de définir la longueur d'exécution à zéro, ce comportement doit faire partie de la Dataclasse. Datan'a jamais été un record stupide. Vous en avez déjà fait un hybride en ajoutant des setters publics.

Il existe un scénario dans lequel vous pouvez envisager d' assouplir ces responsabilités strictes: si Dataest privé de votre projet et ne fait donc partie d'aucune interface publique, vous pouvez toujours garantir une utilisation correcte de la classe. Cependant, cela place la responsabilité de maintenir la Datacohérence dans tout le code, plutôt que de les collecter à un emplacement central.

J'ai récemment écrit une réponse sur l'encapsulation , qui approfondit ce qu'est l'encapsulation et comment vous pouvez vous en assurer.

Le fait que handleTrade()et updateRun()toujours se produisent ensemble (et la deuxième méthode est en fait sur le visiteur et appelle plusieurs autres méthodes sur l'objet de données) sent le couplage temporel . Cela signifie que vous devez appeler les méthodes dans un ordre spécifique, et je suppose que le fait d'appeler des méthodes dans le pire cassera quelque chose au pire, ou ne fournira pas au mieux un résultat significatif. Pas bon.

En règle générale, la bonne façon de refactoriser cette dépendance est que chaque méthode renvoie un résultat qui peut soit être introduit dans la méthode suivante, soit être appliqué directement.

Ancien code:

MyObject x = ...;
x.actionOne();
x.actionTwo();
String result = x.actionThree();

Nouveau code:

MyObject x = ...;
OneResult r1 = x.actionOne();
TwoResult r2 = r1.actionTwo();
String result = r2.actionThree();

Cela présente plusieurs avantages:

• Il déplace des préoccupations distinctes dans des objets distincts ( SRP ).
• Il supprime le couplage temporel: il est impossible d'appeler des méthodes dans le désordre, et les signatures de méthode fournissent une documentation implicite sur la façon de les appeler. Avez-vous déjà regardé la documentation, vu l'objet que vous voulez et travaillé en arrière? Je veux l'objet Z. Mais j'ai besoin d'un Y pour obtenir un Z. Pour obtenir un Y, j'ai besoin d'un X. Aha! J'ai un W, qui est nécessaire pour obtenir un X. Enchaînez tout ensemble, et votre W peut maintenant être utilisé pour obtenir un Z.
• La division d'objets comme celui-ci est plus susceptible de les rendre immuables, ce qui présente de nombreux avantages au-delà de la portée de cette question. La conclusion rapide est que les objets immuables ont tendance à conduire à un code plus sûr.

De mon point de vue, une classe devrait contenir "des valeurs d'état (variables membres) et des implémentations de comportement (fonctions membres, méthodes)".

Les "structures de données hybrides malheureuses" émergent si vous rendez publiques des variables de membre d'état de classe (ou leurs getters / setters) qui ne devraient pas être publiques.

Je ne vois donc pas besoin d'avoir des classes séparées pour les objets de données et les objets de travail.

Vous devriez pouvoir garder les variables de membre d'état non publiques (votre couche de base de données devrait être capable de gérer les variables de membre non publiques)

Une fonctionnalité envie est une classe qui utilise excessivement les méthodes d'une autre classe. Voir Code_smell . Avoir une classe avec des méthodes et des états éliminerait cela.