Рекомендации по рефакторингу: карты в POJO

Question

В настоящее время я являюсь частью проекта, в котором есть такой интерфейс:

public interface RepositoryOperation {

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

В этом интерфейсе около 100 реализаторов.

Чтобы вызвать исполнителя, нужно выполнитьследующее:

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

Теперь я думаю, что явно что-то не так с общим объявлением <Something, Object>.

В настоящее время это заставляет вызывающего OperationImpl фактическиПрочитайте код операции, чтобы узнать, как построить карту аргументов.(и это даже не худшая из проблем, но я не хочу приводить их все, поскольку они довольно очевидны)

После некоторого обсуждения мне удалось убедить моих коллег позволить мне провести некоторый рефакторинг.

Мне кажется, что самым простым «исправлением» было бы изменение интерфейса следующим образом:

public interface RepositoryOperation {

    public OperationResult execute(OperationParam param);  
}

После того, как все конкретные операции определят (расширят) свой собственный OperationParam и необходимыйаргументы будут видны всем.(что является «нормальным способом» делать подобные вещи, ИМХО)

Итак, как я вижу, поскольку реализации интерфейса довольно много, у меня есть несколько вариантов:

1. Попробуйте изменить интерфейс и переписать все вызовы Операции, чтобы использовать объекты вместо карт.Это кажется самым чистым, но я думаю, что, поскольку операций много, на практике может быть слишком много работы.(Вероятно, ~ 2 недели с тестами)

2. Добавьте дополнительный интерфейс к интерфейсу, например, так:

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

   и исправьте вызовы карты, когда я сталкиваюсь с ними во времяреализация функциональности.

3. Жить с этим (пожалуйста, нет!).

Так что мой вопрос.

Кто-нибудь видитлучший подход для «исправления» карт, и если вы это сделаете, вы бы исправили их методом 1 или 2 или не исправили бы их вообще.

РЕДАКТИРОВАТЬ: Спасибо за отличные ответы.Я бы принял ответы как Макса, так и Ридуйделя, если бы мог, но так как я не могу, я склоняюсь немного больше к ответам Ридуиделя.

Answer 1

Я вижу третий путь.

У вас есть карта из <RepOpParam, Object>. Если я вас правильно понимаю, вас беспокоит тот факт, что нет проверки типов. И, очевидно, это не идеально. Но можно перенести проблему проверки типов со всего параметра (ваш OperationParam) на отдельный RepOpParam. Позвольте мне объяснить это.

Предположим, что ваш RepOpParam интерфейс (который в настоящее время выглядит как интерфейс тегов ) изменен следующим образом:

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

Затем вы можете обновить современный код, заменив старые вызовы на

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

с новыми вызовами на

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

Очевидным дополнительным преимуществом является то, что теперь вы можете иметь значение по умолчанию для вашего параметра, скрытое в самом его определении.

Однако я должен пояснить, что этот третий способ - не что иное, как способ слияния двух ваших операций второго способа в один, с уважением к старому прототипу кода, при добавлении в него новых возможностей. Как следствие, я лично пошел бы первым путем и переписал бы все эти «вещи», используя современные объекты (кроме того, подумайте о взгляде на библиотеки конфигурации, которые могут привести вас к интересным ответам на эту проблему).

Answer 2

Прежде всего, я думаю, что интерфейс не идеален. Вы можете добавить некоторые обобщения, чтобы сделать его красивее:

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

Теперь нам понадобится код обратной совместимости. Я бы пошел с этим:

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

Вот как будет выглядеть старая операция:

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

Новая операция будет красивее:

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

Но вызывающий код может теперь использовать обе функции (пример кода):

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

В случае, если операция была старой - она ​​будет использовать метод преобразователя из DeprecatedRepositoryOperation Если операция новая, она будет использовать новую функцию public R execute(T params).

Answer 3

Похоже, у вас есть ненужная и ошибочная абстракция. Каждый раз, когда я вижу интерфейс с одним методом, я думаю, что шаблон стратегии или шаблон действия зависит от того, принимаете ли вы решение во время выполнения или нет.

Один из способов очистки кода состоит в том, чтобы каждая реализация RepositoryOperation имела конструктор, который принимает конкретные аргументы, необходимые для правильной работы метода execute. Таким образом, нет беспорядочного приведения значений объектов на карту.

Если вы хотите сохранить сигнатуру метода execute, вы можете использовать универсальные шаблоны для более точных границ значений Map.