Refactoring Beratung: Karten POJOs

Question

ich derzeit Teil eines Projektes ist, wo es eine Schnittstelle ist wie folgt:

public interface RepositoryOperation { 

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

Diese Schnittstelle hat etwa ~ 100 Implementierer.

einen Implementierer zu nennen, man folgend tun muss:

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

Jetzt denke ich, dass es mit irgendetwas mit einer <Something, Object> allgemeinen Erklärung ist offensichtlich etwas falsch.

Momentan bewirkt dies, dass ein Aufrufer von OperationImpl den Code der Operation tatsächlich lesen muss, um zu wissen, wie die Argument-Map erstellt wird. (und das ist nicht einmal das schlimmste Problem, aber ich möchte nicht alle aufzählen, da sie ziemlich offensichtlich sind)

Nach einiger Diskussion gelang es mir, meine Kollegen davon zu überzeugen, mich etwas Refactoring machen zu lassen.

Es scheint mir, dass die einfachste ‚Lösung‘ wie so die Schnittstelle zu ändern wäre:

public interface RepositoryOperation { 

    public OperationResult execute(OperationParam param); 
} 

Nach all den konkreten Operationen ihrer eigene OperationParam wird definieren (erweitern) und die benötigten Argumente sichtbar wären für jeden. (Das ist die ‚normale Art und Weise‘ Dinge wie die IMHO zu tun)

So wie ich es, da die Schnittstelle Implementierer sehen recht zahlreich sind habe ich mehrere Möglichkeiten:

1. Versuchen Sie, die Schnittstelle und Umschreiben zu ändern Alle Operationen rufen auf, um Objekte anstelle von Maps zu verwenden. Dies scheint am saubersten zu sein, aber ich denke, da die Operationen viel sind, könnte es in der Praxis zu viel Arbeit sein. (~ 2 Wochen mit Tests wahrscheinlich)

2. eine zusätzliche Methode zur Schnittstelle hinzufügen wie folgt:

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

   und fixieren Sie die Karte Anrufe, wenn ich über sie kommen während Funktionalität Umsetzung.

3. Lebe damit (bitte nicht!).

Also meine Frage ist.

Wer sieht einen besseren Ansatz zum "Fixieren" der Karten und wenn Sie das tun würden, würden Sie sie mit Methode 1 oder 2 reparieren oder gar nicht reparieren.

EDIT: Danke für die tollen Antworten. Ich würde sowohl Max 'als auch Riduidels Antworten akzeptieren, wenn ich könnte, aber da ich nicht mehr zu Riduidels neigen kann.

Answer 1

Ich kann einen dritten Weg sehen.

Sie haben eine Karte von <RepOpParam, Object> gemacht. Wenn ich Sie richtig verstehe, stört Sie die Tatsache, dass es keine Typprüfung gibt. Und natürlich ist es nicht ideal. Es ist jedoch möglich, das Problem der Typprüfung vom gesamten Parameter (OperationParam) auf die individuelle RepOpParam zu verschieben. Lass es mich erklären.

Angenommen, Ihre RepOpParam Schnittstelle (die zur Zeit wie ein tagging interface scheint) als sie geändert:

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

Sie können dann modernen Code aktualisieren, indem alte Anrufe

ersetzt

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

mit neuen Anrufen

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

Der offensichtliche kollaterale Vorteil ist Sie können jetzt einen Standardwert für Ihren Parameter haben, der in seiner Definition versteckt ist.

Ich muss jedoch klarstellen, dass dieser dritte Weg nichts anderes als eine Möglichkeit ist, Ihre beiden Operationen des zweiten Wegs in nur einen zusammenzufassen, wobei alter Code-Prototyp respektiert wird, während neue Mächte darin hinzugefügt werden. Als Konsequenz würde ich persönlich den ersten Weg gehen und all dieses "Zeug" umschreiben, indem ich moderne Objekte benutze (außerdem denke über Konfigurationsbibliotheken nach, die dich zu interessanten Answers zu diesem Problem führen können).

Answer 2

Es klingt wie Sie haben eine unnötige und fehlgeleitete Abstraktion. Immer wenn ich eine Schnittstelle mit einer Methode sehe, denke ich an Strategie- oder Aktionsmuster, je nachdem, ob Sie die Entscheidung zur Laufzeit treffen oder nicht.

Eine Möglichkeit, den Code zu bereinigen, besteht darin, dass jede RepositoryOperation-Implementierung einen Konstruktor hat, der die spezifischen Argumente für die korrekte Ausführung der execute-Methode übernimmt. Auf diese Weise werden die Objektwerte in der Karte nicht unordentlich umgewandelt.

Wenn Sie die Signatur der Ausführungsmethode beibehalten möchten, können Sie möglicherweise Generics verwenden, um die Werte der Map enger zu begrenzen.

Answer 3

Zunächst einmal denke ich, dass die Schnittstelle nicht perfekt ist. Sie könnten einige Generika hinzufügen, um es hübscher zu machen:

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

Jetzt werden wir einige Rückwärtskompatibilitätscode benötigen. Ich würde mit dieser gehen:

//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); 
    } 
} 

Hier ist, wie wird alt Betrieb wie folgt aussehen:

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

New Betrieb schönere sein wird:

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

Aber der Aufrufcode können beide Funktionen jetzt (ein Beispiel für Code):

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

Wenn die Operation alt war, wird die Konvertermethode von DeprecatedRepositoryOperation verwendet. Falls der Vorgang ein neuer Vorgang ist, wird die neue Funktion public R execute(T params) verwendet.