Comment refactoriser une application avec plusieurs boîtiers de commutation?

J'ai une application qui prend un entier en entrée et basée sur les appels d'entrée des méthodes statiques de différentes classes. Chaque fois qu'un nouveau numéro est ajouté, nous devons ajouter un autre cas et appeler une méthode statique différente d'une classe différente. Il y a maintenant 50 cas dans le commutateur et chaque fois que j'ai besoin d'ajouter un autre cas, je frémis. Y a-t-il une meilleure manière de faire cela.

J'ai réfléchi et j'ai eu cette idée. J'utilise le modèle de stratégie. Au lieu d'avoir un boîtier de commutation, j'ai une carte des objets de stratégie avec la clé étant l'entier d'entrée. Une fois la méthode invoquée, elle recherchera l'objet et appellera la méthode générique de l'objet. De cette façon, je peux éviter d'utiliser la construction de boîtier de commutation.

Qu'est-ce que tu penses?

Il y a maintenant 50 cas dans le commutateur et chaque fois que j'ai besoin d'ajouter un autre cas, je frémis.

J'adore le polymorphisme. J'adore SOLID. J'adore la programmation orientée objet pure. Je déteste voir ceux-ci donnés une mauvaise réputation parce qu'ils sont appliqués de manière dogmatique.

Vous n'avez pas fait de bons arguments en faveur d'une refactorisation de la stratégie. Le refactoring a d'ailleurs un nom. Cela s'appelle Remplacer le conditionnel par le polymorphisme .

J'ai trouvé quelques conseils pertinents pour vous sur c2.com :

Cela n'a de sens que si les tests conditionnels identiques ou très similaires sont répétés souvent. Pour des tests simples et rarement répétés, le remplacement d'un conditionnel simple par la verbosité de plusieurs définitions de classe, et le déplacement probable de tout cela loin du code qui nécessite réellement l'activité conditionnellement requise, résulterait en un exemple classique d'obscurcissement de code. Préférez la clarté à la pureté dogmatique. - DanMuller

Vous avez un interrupteur avec 50 cas et votre alternative est de produire 50 objets. Oh et 50 lignes de code de construction d'objet. Ce n'est pas un progrès. Pourquoi pas? Parce que ce refactoring ne fait rien pour réduire le nombre de 50. Vous utilisez ce refactoring lorsque vous trouvez que vous devez créer une autre instruction switch sur la même entrée ailleurs. C'est à ce moment que cette refactorisation est utile car elle transforme 100 en 50.

Tant que vous faites référence à "l'interrupteur" comme si c'était le seul que vous ayez, je ne le recommande pas. Le seul avantage de la refactorisation maintenant est qu'elle réduit les chances que certains goofball copient et collent votre commutateur 50 cases.

Ce que je recommande, c'est d'examiner attentivement ces 50 cas afin de trouver des points communs qui peuvent être pris en compte. Je veux dire 50? Vraiment? Vous êtes sûr d'avoir besoin de tant de cas? Vous essayez peut-être de faire trop ici.

Une carte des objets de stratégie seuls, qui est initialisée dans une fonction de votre code, où vous avez plusieurs lignes de code ressemblant à

     myMap.Add(1,new Strategy1());
     myMap.Add(2,new Strategy2());
     myMap.Add(3,new Strategy3());

exige que vous et vos collègues implémentiez les fonctions / stratégies à appeler dans des classes distinctes, de manière plus uniforme (car vos objets de stratégie devront tous implémenter la même interface). Un tel code est souvent un peu plus complet que

     case 1:
          MyClass1.Doit1(someParameters);
          break;
     case 2:
          MyClass2.Doit2(someParameters);
          break;
     case 3:
          MyClass3.Doit3(someParameters);
          break;

Cependant, il ne vous libérera toujours pas du fardeau de la modification de ce fichier de code chaque fois qu'un nouveau numéro doit être ajouté. Les avantages réels de cette approche sont différents:

• l'initialisation de la carte devient maintenant séparé du code d'expédition qui fait appelle la fonction associée à un numéro spécifique, et celui - ci ne pas contenir ces 50 répétitions plus, il suffit de regarder comme myMap[number].DoIt(someParameters). Ainsi, ce code de répartition n'a pas besoin d'être touché chaque fois qu'un nouveau numéro arrive et peut être mis en œuvre selon le principe Open-Closed. De plus, lorsque vous obtenez des exigences où vous devez étendre le code d'expédition lui-même, vous n'aurez plus à changer 50 emplacements, mais un seul.

• le contenu de la carte est déterminé au moment de l'exécution (tandis que le contenu de la construction du commutateur est déterminé avant la compilation), ce qui vous donne la possibilité de rendre la logique d'initialisation plus flexible ou extensible.

Alors oui, il y a quelques avantages, et c'est sûrement un pas vers plus de code SOLIDE. Si cela rapporte au refactorisateur, c'est quelque chose que vous ou votre équipe devrez décider par vous-même. Si vous ne vous attendez pas à ce que le code de répartition soit modifié, que la logique d'initialisation soit modifiée et que la lisibilité de la switchne soit pas un réel problème, alors votre refactoring pourrait ne pas être si important maintenant.

Je suis fortement en faveur de la stratégie décrite dans la réponse de @DocBrown .

Je vais suggérer une amélioration de la réponse.

Les appels

 myMap.Add(1,new Strategy1());
 myMap.Add(2,new Strategy2());
 myMap.Add(3,new Strategy3());

peut être distribué. Vous n'avez pas besoin de revenir au même fichier pour ajouter une autre stratégie, qui adhère encore mieux au principe Open-Closed.

Supposons que vous implémentiez le Strategy1fichier Strategy1.cpp. Vous pouvez contenir le bloc de code suivant.

namespace Strategy1_Impl
{
   struct Initializer
   {
      Initializer()
      {
         getMap().Add(1, new Strategy1());
      }
   };
}
using namespace Strategy1_Impl;

static Initializer initializer;

Vous pouvez répéter le même code dans chaque fichier StategyN.cpp. Comme vous pouvez le voir, ce sera beaucoup de code répété. Pour réduire la duplication de code, vous pouvez utiliser un modèle qui peut être placé dans un fichier accessible à toutes les Strategyclasses.

namespace StrategyHelper
{
   template <int N, typename StrategyType> struct Initializer
   {
      Initializer()
      {
         getMap().Add(N, new StrategyType());
      }
   };
}

Après cela, la seule chose que vous devez utiliser dans Strategy1.cpp est:

static StrategyHelper::Initializer<1, Strategy1> initializer;

La ligne correspondante dans StrategyN.cpp est:

static StrategyHelper::Initializer<N, StrategyN> initializer;

Vous pouvez amener l'utilisation des modèles à un autre niveau en utilisant un modèle de classe pour les classes de stratégie concrètes.

class Strategy { ... };

template <int N> class ConcreteStrategy;

Et puis, au lieu de Strategy1, utilisez ConcreteStrategy<1>.

template <> class ConcreteStrategy<1> : public Strategy { ... };

Modifiez la classe d'assistance pour enregistrer Strategys en:

namespace StrategyHelper
{
   template <int N> struct Initializer
   {
      Initializer()
      {
         getMap().Add(N, new ConcreteStrategy<N>());
      }
   };
}

Modifiez le code dans Strateg1.cpp en:

static StrategyHelper::Initializer<1> initializer;

Changez le code dans StrategN.cpp en:

static StrategyHelper::Initializer<N> initializer;