Дублирование кода: Следует ли использовать отдельный метод или шаблон компоновщика?

#java #spring #inheritance #design-patterns #interface

Вопрос:

У меня есть следующие 2 метода (некоторые коды опущены для краткости) в моем приложении Java (Spring), и я хочу уменьшить количество дублирующихся блоков в этих методах. На данный момент, после проведения некоторых исследований, некоторые люди предлагают шаблон конструктора, некоторые из них используют другой аналогичный метод шаблона. Я также думаю, что я мог бы просто создать 2 отдельных метода и переместить каждый повторяющийся блок кода в эти методы.

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

Нет: я опустил свой код и для краткости использую простой код:

 @Override
public MultipartFile exportAaaaa() throws IOException {

    // repeated code block I
    workbook = new XSSFWorkbook();
    sheet = workbook.createSheet(TextBundleUtil.read(TITLE));
    rowCount = new AtomicInteger(0);    
    //

    // private block to this method
    final Page<Aaaaa> page = aaaaaService.findAll());

    // ...

    // repeated code block II
    outputFile = File.createTempFile(TextBundleUtil.read(TITLE), EXTENSION);
    try (FileOutputStream outputStream = new FileOutputStream(outputFile)) {
        workbook.write(outputStream);
    } catch (IOException e) {
        LoggingUtils.error("Writing is failed ", e);
    }
    final FileInputStream input = new FileInputStream(outputFile);

    final String fileName = TextBundleUtil.read(TITLE).concat(EXTENSION);
    return new MockMultipartFile(fileName,
            fileName, CONTENT_TYPE, IOUtils.toByteArray(input));
    //
}
 
 @Override
public MultipartFile exportBbbbb() throws IOException {

    // repeated code block I
    workbook = new XSSFWorkbook();
    sheet = workbook.createSheet(TextBundleUtil.read(TITLE));
    rowCount = new AtomicInteger(0);    
    //

    // private block to this method
    final Page<Bbbbb> page = bbbbbService.findBy Uuid(uuid));

    // ...

    // repeated code block II
    outputFile = File.createTempFile(TextBundleUtil.read(TITLE), EXTENSION);
    try (FileOutputStream outputStream = new FileOutputStream(outputFile)) {
        workbook.write(outputStream);
    } catch (IOException e) {
        LoggingUtils.error("Writing is failed ", e);
    }
    final FileInputStream input = new FileInputStream(outputFile);

    final String fileName = TextBundleUtil.read(TITLE).concat(EXTENSION);
    return new MockMultipartFile(fileName,
            fileName, CONTENT_TYPE, IOUtils.toByteArray(input));
    //
}
 

Комментарии:

1. Я предлагаю опубликовать ваш фактический, рабочий код для проверки кода .

Ответ №1:

Я вижу здесь, что дублированный код является журналированием(или консольной печатью). Но в этом примере неясно, что делают AService и bService? Может быть, у них внутри другая логика? Как я вижу, они возвращают другой результат. Но если A.class и B.class являются реализацией одного интерфейса, а AService и bService также являются реализациями одного интерфейса. Это может быть единственный способ. Может быть)

Комментарии:

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

2. Чико? Любой ответ, пожалуйста?

Ответ №2:

Если aaaaaService и bbbbbService имеют общий интерфейс, например

 interface PageService<T> {
    Page<T> findAll();
}
 

тогда мы можем использовать один метод с аргументами типа.

 private <T> MultipartFile export(Service<T> service) throws IOException {
    // repeated code block I
    final Page<T> page = service.findAll());
    // repeated code block II
}
 

Мы можем либо объявить этот метод как public , либо сохранить предыдущие методы в качестве методов покрытия, что зависит от того, хотите ли вы, чтобы клиенты обрабатывали/получали доступ к базовой службе, и от того, можете ли вы изменить интерфейс или суперкласс, указанный @Override .

 public MultipartFile exportAaaaa() throws IOException {
    // assuming that 'aaaaaService' is a class member
    return export(aaaaaService);
}
 

Если нет общего интерфейса, я бы просто извлек общие блоки кода для разделения методов.

 private MultipartFile exportAaaaa() throws IOException {
    prepareWorkbook(/* parameters */);
    final Page<T> page = aaaaaService.findAll());
    constructOutput(/* parameters */);
}

private /* return type */ prepareWorkbook(/* parameters */) {
    // repeated code block I
}

private /* return type */ constructOutput(/* parameters */) {
    // repeated code block II
}
 

Комментарии:

1. Большое спасибо за ваши ценные объяснения. Но главное, на чем я сосредоточился, — это повторяющиеся части кода, а не сервис. Итак, игнорируйте службу (предположим, что они полностью делают несвязанные вещи) и обновите свой ответ, предложив правильный способ. Должен ли я извлечь 2 повторяющиеся части из 2 отдельных методов и вызвать их в этих exportAaaaa и exportBbbbb методах?

2. @Jonathan Я рассмотрел обе альтернативы в своем ответе. Если есть общий интерфейс, вы можете его использовать. Однако, если вы не хотите полагаться на службу, выполняющую конкретный контракт, вы можете выбрать второй подход, который я выделил, — это извлечение двух дублированных частей в специальные методы.

Ответ №3:

Похоже, что дублирование в вашем коде может быть довольно легко извлечено в отдельные методы:

 private void doCodeBlockI() {
    // repeated code block I
    workbook = new XSSFWorkbook();
    sheet = workbook.createSheet(TextBundleUtil.read(TITLE));
    rowCount = new AtomicInteger(0);    
}    

private MultipartFile doCodeBlockII() {
    // repeated code block II
    outputFile = File.createTempFile(TextBundleUtil.read(TITLE), 
    EXTENSION);
    try (FileOutputStream outputStream = new FileOutputStream(outputFile)) {
        workbook.write(outputStream);
    } catch (IOException e) {
        LoggingUtils.error("Writing is failed ", e);
    }
    final FileInputStream input = new FileInputStream(outputFile);

    final String fileName = TextBundleUtil.read(TITLE).concat(EXTENSION);
    return new MockMultipartFile(fileName,
            fileName, CONTENT_TYPE, IOUtils.toByteArray(input));
}    

@Override
public MultipartFile exportAaaaa() throws IOException {
    doCodeBlockI();

    // private block to this method
    final Page<Aaaaa> page = aaaaaService.findAll();

    // ...

    return doCodeBlockII();
}

@Override
public MultipartFile exportBbbbb() throws IOException {
    doCodeBlockI();

    // private block to this method
    final Page<Bbbbb> page = bbbbbService.findByUuid(uuid);

    // ...

    return doCodeBlockII();
}
 

Другим способом достижения этой цели было бы использование абстрактного класса:

 public abstract class Exporter {

    public MultipartFile export() throws IOException {

        // repeated code block I
        workbook = new XSSFWorkbook();
        sheet = workbook.createSheet(TextBundleUtil.read(TITLE));
        rowCount = new AtomicInteger(0);    
        //

        // private block will be called inside the abstract method
        doPrivateBlock();
    
        // repeated code block II
        outputFile = File.createTempFile(TextBundleUtil.read(TITLE), EXTENSION);
        try (FileOutputStream outputStream = new FileOutputStream(outputFile)) {
            workbook.write(outputStream);
        } catch (IOException e) {
            LoggingUtils.error("Writing is failed ", e);
        }
        final FileInputStream input = new FileInputStream(outputFile);

        final String fileName = TextBundleUtil.read(TITLE).concat(EXTENSION);
        return new MockMultipartFile(fileName,
            fileName, CONTENT_TYPE, IOUtils.toByteArray(input));
        //
    }

    protected abstract void doPrivateBlock();
}
 

Затем расширьте его дважды, реализовав doPrivateBlock() метод:

 public class AaaaaExporter extends Exporter {

    AaaaaService aaaaaService; // need to instantiate this

    public AaaaaExporter() {}

    @Override
    private void doPrivateBlock() {
        // private block to this method
        final Page<Aaaaa> page = aaaaaService.findAll());

        // ...
    }
}

public class BbbbbExporter extends Exporter {

    BbbbbService bbbbbService; // need to instantiate this

    public BbbbbExporter() {}

    @Override
    private void doPrivateBlock() {
        // private block to this method
            final Page<Bbbbb> page = bbbbbService.findByUuid(uuid));

        // ...
    }
}
 

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

Комментарии:

1. Большое спасибо, это, кажется, правильное и самое простое решение. С другой стороны, как насчет того, чтобы другие классы также имели эти doCodeBlockI и doCodeBlockII ? В этом случае, как я должен переместить эти методы ( doCodeBlockI и doCodeBlockII ) и использовать их в этом методе и из других классов экспорта? Должен ли я сделать их ( doCodeBlockI и doCodeBlockII ) статическим методом и вызвать из этого класса? Или есть какая — то необходимость использовать наследование?

2. Это действительно зависит от ваших конкретных потребностей, но да, вспомогательный класс может помочь (вы также можете передавать службы по мере необходимости в качестве параметров, и эти службы могут вести себя по-разному в зависимости от контекста (например Printer , vs CloudPrinter vs DummyPrinter все, реализующие метод IPrinter s print() ).

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

4. Какой-нибудь ответ, пожалуйста?