Как реорганизовать большой класс, использующий стратегии?

Проблема

У меня есть большой класс (около 1500 LOC), и он использует разные «стратегии» для преобразования данных из одного объекта в другой. У меня есть представление этого класса:

public class FooService implements FooProcessing {
    FooRequestTransformer fooRequestTransformer;
    AnotherService anotherService;
    InstanceVar1 iVar1;
    InstanceVar2 iVar2;    
...

Существует интерфейс (внешний по отношению к классу), который использует этот класс:

interface TransformerStrategy {
    public FooRequest transform(FooResponse response);
}

Что передается в этот метод (внутри класса FooService):

private FooResponse getResponse(FooResponse fooResponse, TransformerStrategy transformerStrategy) {
    FooRequest fooRequest = transformerStrategy.transform();
    fooResponse = anotherService.bar(fooRequest); 
    return fooResponse;
}

Здесь находится точка входа, которая использует метод getResponse() и анонимно создает TransformerStrategy:

public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...){
    FooResponse fooResponse = anotherService.baz(fooRequest);

    TransformerStrategy myTransformerStrategy = new TransformerStrategy() {
        public FooRequest transform(FooResponse fooResponse) {
            fooRequestTransformer.transform(param1, param2, iVar1, iVar2) 
        }
    }

    FooResponse fooResponse = getResponse(fooResponse, myTransformerStrategy);
    ...//other code
}

Теперь проблема в следующем: есть несколько методов, таких как getFooForSomeFlow1() (внутри FooService), которые имеют собственную анонимную реализацию TransformerStrategy и впоследствии вызывают getResponse(). Как вы можете себе представить, это очень запутанно, а также сбивает с толку при отладке (т. е. вы входите в getResponse(), а затем внезапно возвращаетесь в getFooForSomeFlow1())

Возможное решение

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

protected class StrategyProvider {
    public ABCTransformerStrategy newABCTransformerStrategy(FooRequestTransformer transformer, Param1 param1, Param2 param2) {
        return new ABCTransformerStrategy(transformer, param1, param2);
    }
}

protected class ABCTransformerStategy implements TransformerStrategy {
    protected FooRequestTransformer transformer;
    protected Param1 param1; 
    protected Param2 param2;

    //...constructor here

    //...overridden transform method as above
}

В одном из комментариев говорится: «Преобразование анонимного класса во внутренний класс для целей тестирования». Однако они преобразовали только один из них, а остальные оставили. Получается, что они начали процесс рефакторинга и остановились посередине.

Итак, я подумал, что могу завершить процесс рефакторинга и переместить все анонимные классы во внутренние классы, а затем, наконец, переместить эти классы и StrategyProvider во внешние классы. Проблема в том, что «преобразование анонимного во внутреннее» добавляет больше шаблонного кода (см. ABCTransformerStrategy выше; я должен передать все данные в конструктор), и я не совсем уверен, сколько я выиграю, выполняя этот процесс рефакторинга.

У меня есть два вопроса:

  1. Должен ли я продолжать этот подход?
  2. Или есть другой шаблон проектирования, который я могу применить, который будет более подходящим и действительно упростит этот код?

Спасибо


person Atif    schedule 06.03.2013    source источник
comment
Извините, удалил, когда понял.   -  person christopher    schedule 07.03.2013
comment
np, я удалил свой комментарий и немного прояснил проблему   -  person Atif    schedule 07.03.2013


Ответы (5)


Судя по предоставленному вами образцу кода, он на первый взгляд сложен. Почему бы просто не

public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...)
{
    FooResponse fooResponse = anotherService.baz(fooRequest);

    FooRequest  fooRequest = fooRequestTransformer
                                 .transform(param1, param2, iVar1, iVar2); 

    FooResponse fooResponse2 = anotherService.bar(fooRequest);
    ...//other code
}

Если только не происходит что-то еще, чего вы нам не показали.

person irreputable    schedule 06.03.2013

Ваш класс провайдера на самом деле представляет собой шаблон Factory, что я и собирался предложить.

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

Просто из личных предпочтений я бы использовал один метод, используемый для возврата экземпляров, который принимает значение int в качестве дискриминатора. Эти int могут быть статическими конечными переменными внутри класса провайдера, например:

public static final int ABCTransformerStrategy = 1;

Кроме того, сделав класс Factory абстрактным, вы избавитесь от необходимости создавать экземпляр класса каждый раз, когда вам нужно его использовать, что сделает код намного чище.

Изменить

Как было предложено, использование Enum является гораздо более желательным и семантически правильным способом перевода значений в конкретные классы, которые они представляют.

person christopher    schedule 06.03.2013
comment
Предпочтительной альтернативой использованию int является использование Enum для определения того, какой конкретный экземпляр необходимо создать. - person nattyddubbs; 07.03.2013
comment
Согласованный. Я учту свой ответ сейчас :) - person christopher; 07.03.2013

Если вы хотите предоставить набор связанных объектов, вы всегда можете использовать класс Abstract Factory. Это не простой шаблон дизайна, но он наиболее подходит для вас.

Взгляните на это: Abstract Factory

person Giovani Guizzo    schedule 06.03.2013

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

person cfeduke    schedule 06.03.2013

Я попробовал подход, предложенный неуважаемым, и попробовал «Встроенный метод» в методе getResponse(), но он встраивал некоторый повторяющийся код (именно поэтому мы в первую очередь извлекли этот повторяющийся код в getResponse()). Я должен был уточнить в своем вопросе, что в getResponse() кода больше, чем то, что я показал.
Что касается создания Фабрики, я не мог оправдать добавление в класс большего количества шаблонов и LOC, тем более что мне нужно пройти много данных в фабричный метод и/или внутренний класс.

Вместо этого мы обернули анонимные внутренние классы таким методом:

private TransformerStrategy createTransformerStrategyWithABCValues(Param1 param1, Param2 param2, IVar ivar, IVar1 ivar2) {
    return new TransformerStrategy() {
        public FooRequest transform(FooResponse response) {
            return FooRequestTransformer.transform(
                    param1,
                    param2,
                    iVar1,
                    iVar2);
        }
    };
}

Теперь метод вызова выглядит так:

public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...){
    FooResponse fooResponse = anotherService.baz(fooRequest);
    TransformerStrategy myTransformerStrategy = createTransformerStrategyWithABCValues(param2, param2, iVar1, iVar2);
    FooResponse fooResponse = getResponse(fooResponse, myTransformerStrategy);
    ...//other code
}

В процессе этого мы обнаружили, что 5 стратегий имеют некоторое дублирование, поэтому нам удалось сократить количество стратегий до 3. Мы избавились от класса StrategyProvider, а также от предоставленного внутреннего класса.

С таким подходом стало несколько легче отлаживать/обслуживать, и мы смогли избавиться от дублирования кода.

person Atif    schedule 11.03.2013