J'ai une question concernant la duplication et le refactorisation du code, espérons que ce n'est pas trop général. Dites que vous avez un code plutôt petit (~ 5 lignes) qui est une séquence d'invocations de fonction qui n'est pas un niveau très bas . Ce code est répété à plusieurs endroits, il serait donc probablement une bonne idée d'extraire une méthode ici. Cependant, dans cet exemple particulier, cette nouvelle fonction souffrirait de la faible cohésion (qui se manifeste, entre autres, en ayant du mal à trouver un bon nom pour la fonction). La raison en est probablement parce que ce code répété ne fait qu'une partie d'un algorithme plus important - et il est difficile de la diviser en étapes bien nommées. Que suggéreriez-vous dans un tel scénario? EDIT: Je voulais conserver la question sur un niveau général, de sorte que plus de gens peuvent potentiellement le trouver utiles, mais il serait évidemment préférable de la sauvegarder avec un échantillon de code. L'exemple n'est peut-être pas le meilleur possible (ça sent assez de façons), mais j'espère que cela fait son travail:
5 Réponses :
question est trop générale pour dire vraiment, mais comme exercice:
Supposons que vous résumez cela. Pensez à ce que les raisons probables sont destinées à modifier la fonction de 5 lignes résultante. Voulez-vous probablement apporter des modifications qui s'appliquent à tous les utilisateurs ou devriez-vous avoir à écrire une nouvelle fonction légèrement différente de l'ancien, chaque fois que certains appelants ont une raison de vouloir un changement?
Si vous souhaitez le modifier pour tous les utilisateurs, c'est une abstraction viable. Donnez-lui un mauvais nom maintenant, vous pourriez penser à un meilleur plus tard.
Si vous allez finir par diviser cette fonction dans de nombreuses versions similaires que votre code évolue à l'avenir, ce n'est probablement pas une abstraction viable. Vous pouvez toujours écrire la fonction, mais c'est plus d'une "fonction d'assistance" en économie de code que celle de votre modèle formel du problème. Ce n'est pas très satisfaisant: la répétition de cette quantité de code est un peu inquiétante, car elle le suggère devrait être une abstraction viable à y avoir quelque part.
Peut-être que 4 des 5 lignes pourraient être abstraits, car ils sont plus cohérents et la cinquième ligne tout simplement à pendre avec eux en ce moment. Ensuite, vous pouvez écrire 2 nouvelles fonctions: une nouvelle abstraction, et l'autre est juste une aide qui appelle la nouvelle fonction, puis exécute la ligne 5. Une de ces fonctions pourrait alors avoir une durée de vie utile attendue plus longue que l'autre .. .
J'essaie vraiment de trouver un morceau de code compact pour sauvegarder ma question avec, mais la base de code que je me suis tellement contestée au refacteur, semble être un tel désordre qu'il est difficile d'isoler le problème et il serait probablement difficile de coller n'importe quoi sans entrer dans une description longue et ennuyeuse. Je pense que vous avez cloué avec le "il devrait y avoir une abstraction viable là-bas quelque part" suggestion. C'est un sentiment si fort que je continue toujours de regarder le code, refusant de lâcher prise. Car je
J'ai ajouté des échantillons de code - souhaitez-vous augmenter la description de l'approche générale avec des conseils et des suggestions spécifiques - pour une meilleure illustration?
@ Lukem00: Je ne sais pas, peut-être que ces 3 lignes communes pourraient être transformées en une méthode de Socketaction , également appelée OnLoginCorrection ? C'est-à-dire que chaque loginhandler devrait faire sa chose, puis déléguer sur le sockettaction , qui fait cette session.
@ Lukem00: De plus, difficile à dire sans regarder le reste du programme, mais une classe interne statique privée avec une seule fonction, qui prend comme paramètre un objet de la classe extérieure, pourrait être un candidat à être un intérieur non statique classe (donc il contient une référence à l'objet qui l'a créé). Je ne sais pas si cela ferait une différence pour cette question spécifique du code répété, cependant.
Déléguer à Socketaction n'est peut-être pas la solution la plus propre, mais je suppose que cela pourrait faire l'affaire. Ce serait une délégation conditionnelle si (en particulier Loginhandler La mise en œuvre décide), n'est-ce pas légèrement bizarre? Je serais peut-être paranoïaque ici - j'ai passé beaucoup de temps à expérimenter le code que j'ai gars de mon bon jugement.
Je testais l'idée de remplacer de multiples conditionnels avec un modèle de stratégie - il a permis de séparer différents comportements, mais en même temps laissé le code un peu désordonné. Il a l'air légèrement inutile, surtout que ces stratégies ne peuvent être réutilisées nulle part ailleurs, afin que je puisse les revenir à des méthodes standard et en choisir un avec un commutateur régulier.
D'ailleurs. Désolé pour la longue discussion, je continue à oublier comment les choses fonctionnent. De plus, il est difficile de questions comme ça pour utiliser une simple approche de Q & A.
Je pensais à cela ces derniers temps et je comprends exactement ce que vous obtenez. Il me semble que c'est une abstraction de mise en œuvre plus que tout, et est donc plus agréable si vous pouvez éviter de changer une interface. Par exemple, en C ++, je pourrais extraire la fonction dans le CPP sans toucher l'en-tête. Cela diminue quelque peu l'exigence de l'abstraction de la fonction d'être bien formée et significative à l'utilisateur de la classe, car elle leur est invisible jusqu'à ce qu'ils en aient vraiment besoin (lors de l'addition de la mise en œuvre).
Oui, il s'agit de choses internes afin que l'interface reste la même. Peut-être que c'est une façon utopique de penser, mais je crois toujours même que même des méthodes privées devraient former des abstractions décentes. La duplication de code est une seule question et la lisibilité est une autre. Je souhaite que toutes mes méthodes ressemblent à la méthode de Beck composée et étaient aussi lisibles que Cunningham's novembre (20, 2005) ... :)
Pour moi, le mot opératoire est "seuil". Un autre mot pour cela serait probablement "odeur".
Les choses sont toujours dans un équilibre. On dirait que (dans ce cas) comme le centre d'équilibre est en cohésion (cool); et comme vous n'avez qu'un petit nombre de doublons, il n'est pas difficile de gérer.
Si vous avez eu un "événement" majeur se produisait et que vous êtes allé à des doublons "1000", la balance aurait alors été écrasée et que vous pourriez donc vous approcher.
Pour moi, quelques doublons gérables ne sont pas un signal de refacteur (encore); Mais je garderais un œil dessus.
Ok, c'est est gênant avec ce sujet étant divisé en deux questions - désolé à ce sujet. Je suppose que toute la question pour le code de code que je montre devrait être en fait quelque chose comme "Comment refactoriseriez-vous le tout pour la rendre plus lisible?" - Parce que c'est la vraie question que j'ai ici. Je crains que personne ne le lirait même alors, alors j'ai essayé de reformuler, c'est que cela semble donc pertinent à plus de gens que de moi.
Héritage est votre ami!
Ne pas dupliquer le code. Même si une seule ligne de code est très longue ou difficile, le refacteur à une méthode distincte avec un nom distinctif. Pensez-y comme une personne qui lit votre code dans un an. Si vous nommez cette fonction "BLABLA", le prochain gars savoir quelle est cette fonction sans lire son code? Sinon, vous devez changer le nom. Après une semaine de pensée comme celle-là, vous vous y habituerez et votre code sera
Pour moi, le test LitsMus est: Si je dois modifier cette séquence de code au même endroit (par exemple, ajouter une ligne, modifier la commande), aurais-je besoin de faire le même changement Autres emplacements?
Si la réponse est oui, il s'agit d'une entité logique, "atomique" et doit être refactable. Si la réponse n'est pas non plus, c'est une séquence d'opérations qui se produisent dans plus d'une situation - et si elles sont refactées, vous feront probablement plus de problèmes à l'avenir.