conseil refactorisation: cartes à POJO

Question

Je fais actuellement partie d'un projet où il y a une interface comme ceci:

public interface RepositoryOperation { 

    public OperationResult execute(Map<RepOpParam, Object> params); 
} 

Cette interface a environ ~ 100. Implémenteurs

Pour appeler un implémenteur il faut faire ce qui suit:

final Map<RepOpParam, Object> opParams = new HashMap<RepOpParam, Object>(); 
opParams.put(ParamName.NAME1, val1); 
opParams.put(ParamName.NAME2, val2); 

Maintenant, je pense qu'il ya évidemment quelque chose de mal à quoi que ce soit avec une déclaration générique <Something, Object>.

Actuellement, l'appelant d'un OperationImpl doit réellement lire le code de l'opération pour savoir comment créer la table d'arguments. Après quelques discussions, j'ai réussi à convaincre mes collègues de me laisser refaire.

Il me semble que le plus simple « solution » serait de changer l'interface comme ceci:

public interface RepositoryOperation { 

    public OperationResult execute(OperationParam param); 
} 

Après toutes les opérations concrètes définiront (étendre) leur propre OperationParam et les arguments nécessaires seraient visibles à tous. (Ce qui est la « voie normale » de faire des choses comme ça à mon humble avis)

Alors que je vois depuis les implémenteurs d'interface sont assez nombreux, j'ai plusieurs choix:

1. Essayez de changer l'interface et réécriture tous les appels de l'opération pour utiliser des objets au lieu de cartes. Cela semble le plus propre, mais je pense que puisque les opérations sont nombreuses, cela peut être trop de travail dans la pratique. (~ 2 semaines avec des tests probablement)

2. Ajouter une méthode supplémentaire à l'interface comme ceci:

   public interface RepositoryOperation { 
       public OperationResult execute(Map<String, Object> params); 
       public OperationResult execute(OperationParam params); 
   } 
   

   et fixent les appels carte à chaque fois que je tombe sur eux pendant la mise en œuvre de la fonctionnalité.

3. Vivez avec (s'il vous plaît non!).

Donc ma question est.

Quelqu'un trouve-t-il une meilleure approche pour 'corriger' les cartes et si vous le faites, corrigez-les avec la méthode 1 ou 2 ou ne les corrigez pas du tout.

EDIT: Merci pour les bonnes réponses. J'accepterais les réponses de Max et de Riduidel si je le pouvais, mais puisque je ne peux pas, je me penche un peu plus vers Riduidel.

Answer 1

Je peux voir une troisième voie. Vous avez une carte en <RepOpParam, Object>. Si je vous comprends bien, ce qui vous dérange, c'est le fait qu'il n'y a pas de vérification de type. Et évidemment, ce n'est pas idéal. Mais, il est possible de déplacer le problème de vérification de type de l'ensemble du paramètre (votre OperationParam) à l'individu RepOpParam. Laissez-moi l'expliquer.

Supposons votre interface RepOpParam (qui semble actuellement comme un tagging interface) est modifié comme:

public interface RepOpParam<Value> { 
    public Value getValue(Map<RepOpParam, Object> parameters); 
} 

Vous pouvez ensuite mettre à jour le code moderne en remplaçant les anciens appels à

String myName = (String) params.get(ParamName.NAME1); 

avec de nouveaux appels à

String myName = ParamName.NAME1.getValue(params); 

L'avantage collatéral évident soit Vous pouvez maintenant avoir une valeur par défaut pour votre paramètre, cachée dans sa définition même.

Je dois cependant préciser que cette troisième voie n'est rien de plus qu'un moyen de fusionner vos deux opérations de la deuxième manière en une seule, en respectant l'ancien prototype de code, tout en y ajoutant de nouveaux pouvoirs. En conséquence, je passerais personnellement à la première étape, et réécrirais tout ce "truc", en utilisant des objets modernes (d'ailleurs, pensez à jeter un oeil à la configuration librarires, qui peut vous conduire à des réponses intéressantes à ce problème).

Answer 2

Il semble que vous ayez une abstraction inutile et mal orientée. Chaque fois que je vois une interface avec une méthode, je pense que le modèle de stratégie ou le modèle d'action, selon que vous prenez la décision à l'exécution ou non. Une manière de nettoyer le code consiste à faire en sorte que chaque implémentation de RepositoryOperation possède un constructeur qui prend les arguments spécifiques dont elle a besoin pour exécuter la méthode execute correctement. De cette façon, il n'y a pas de distribution désordonnée des valeurs d'objet dans la carte.

Si vous souhaitez conserver la signature de la méthode d'exécution, vous pouvez utiliser des génériques pour définir des limites plus strictes sur les valeurs de la carte.

Answer 3

Tout d'abord, je pense que l'interface n'est pas parfaite. Vous pouvez ajouter des génériques pour le rendre plus joli:

public interface RepositoryOperation<P extends OperationParam, R extends OperationResult> { 
    public R execute(T params); 
} 

Maintenant, nous aurons besoin d'un code de rétrocompatibilité. Je vais avec ceci:

//We are using basic types for deprecated operations 
public abstract class DeprecatedRepositoryOperation implements RepositoryOperation<OperationParam, OperationResult> { 
    //Each of our operations will need to implement this method 
    public abstract OperationResult execute(Map<String, Object> params); 

    //This will be the method that we can call from the outside 
    public OperationResult execute(OperationParam params) { 
     Map<String, Object> paramMap = getMapFromObj(params); 
     return execute(paramMap); 
    } 
} 

Voici comment vieux look opération comme:

public class SomeOldOperation extends DeprecatedRepositoryOperation { 
    public OperationResult execute(Map<String, Object> params) { 
     //Same old code as was here before. Nothing changes 
    } 
} 

Nouvelle opération sera plus jolie:

public class DeleteOperation implements RepositoryOperation<DeleteParam, DeleteResult> { 
    public DeleteResult execute(DeleteParam param) { 
     database.delete(param.getID()); 
     ... 
    } 
} 

Mais le code appelant peut utiliser à la fois fonctionne maintenant (un exemple de code):

String operationName = getOperationName(); //="Delete" 
Map<String, RepositoryOperation> operationMap = getOperations(); //=List of all operations 
OperationParam param = getParam(); //=DeleteParam 
operationMap.execute(param); 

Si l'opération était ancienne, elle utilisera la méthode de conversion de DeprecatedRepositoryOperation. Si l'opération est nouvelle, elle utilisera la nouvelle fonction public R execute(T params).