Рефакторинг вложенный для циклов

Question

У меня есть ситуация, когда у меня есть родительские дочерние отношения между двумя наборами данных. У меня есть коллекция родительских документов и коллекция дочерних документов. Требование заключается в том, что родители и их соответствующие дети должны быть экспортированы в «PDF-документ». Простая реализация описанной выше ситуации может быть следующей (псевдокод java-ish ниже):

for(Document parentDocument:Documents){
   ExportToPdf(parentDocument);
    for(Document childDocument:parentDocument.children()){
      AppendToParentPdf(childDocument);  
  }
}

Что-то, как указано выше, вероятно, решит проблему, но внезапно требования изменятся, и теперь каждый из этих родителей и их соответствующих детей должен быть в отдельных файлах PDF, поэтому приведенный выше фрагмент кода изменяется путем изменения AppendToParentPdf() ExportToPdf() следует:

for(Document parentDocument:Documents){
   ExportToPdf(parentDocument);
    for(Document childDocument:parentDocument.children()){
      ExportToPdf(childDocument);  
  }
}

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

Мои вопросы к SO:

1. Существуют ли более точные представления об отношениях между родителями и детьми, такие как приведенные выше, где вместо грубого форсирования всех документов и их потомков способом O(n^2) я могу использовать другую структуру данных или техника обхода всей структуры более оптимальным способом.

2. В сценарии, который я описал выше, где бизнес-правила достаточно гибки в отношении способа экспорта pdf-файлов, есть ли более разумный способ кодировать природу функции экспорта? Также формат экспорта является временным. PDF-файлы могут уступать * .docs / csvs / xmls и др.

Будет здорово взглянуть на это.

Спасибо

Answer 1

Существуют ли более точные представления об отношениях между родителями и детьми, такие как приведенные выше, где вместо грубого принуждения ко всем документам и их дочерним элементам обращаются O (n ^ 2).

Это не O(N^2). Это O(N), где N - общее количество родительских и дочерних документов. Предполагая, что ни у одного ребенка нет более одного родительского документа, вы не сможете значительно улучшить производительность. Кроме того, стоимость обхода, вероятно, тривиальна по сравнению со стоимостью вызовов, генерирующих PDF-файлы.

Единственный случай, когда вы можете рассмотреть вопрос об оптимизации, - это если дочерние документы могут быть детьми нескольких родителей. В этом случае вам может понадобиться отслеживать документы, для которых вы уже сгенерировали PDF-файлы ... и пропускать их, если вы повторно зайдете их в обход. Тест «Я уже видел этот документ» может быть реализован с помощью HashSet.

Answer 2

Вы можете инкапсулировать то, что вы хотите сделать с документом в обработчике. Это также позволит вам в будущем определять новые обработчики, которые вы можете передать существующему коду.

interface DocumentHandler {
    void process(Document d);
}

class ExportToPdf implements DocumentHandler { ... }
class AppendToParentPdf implements DocumentHandler { ... }

// Now you're just passing the interface whose implementation does something with the document
void handleDocument(DocumentHandler parentHandler, DocumentHandler childHandler) {
    for(Document parent : documents) {
        parentHandler.process(parent);

        for(Document child : parent.children()) {
            childHandler.process(child);
        }
    }
}

DocumentHandler appendToParent = new AppendToParentPdf();
DocumentHandler exportToPdf = new ExportToPdf();

// pass the child/parent handlers as needed
handleDocument(exportToPdf, appendToParent);
handleDocument(exportToPdf, exportToPdf);

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

Answer 3

Для вашего второго вопроса вы можете использовать шаблон провайдера или его расширение.

Шаблон провайдера. Этот шаблон имеет свои корни в шаблоне Стратегии и позволяет вам проектировать свои данные и поведение в абстракции, чтобы вы могли поменять реализацию в любое время

Answer 4

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

Set<Document> alreadyExported = new HashSet<Document>();

for(Document parentDocument:Documents){
   ExportToPdf(parentDocument);
   for(Document childDocument:parentDocument.children()){
      if(!aldreadyExported.contains(childDocument)){
         ExportToPdf(childDocument);
         alreadyExported.add(childDocument);
      }  
   }
}

Answer 5

Проблема второго вопроса может быть решена простым созданием inteface Exporter с помощью метода export(Document doc); и затем реализует его для каждого из различных форматов, например, class DocExporterImpl implements Exporter.

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

Answer 6

Я пытался вписать это в комментарий, но слишком много сказать ...

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

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

Относительно того, является ли n² проблемой, зависит. Насколько велика? Если вам приходится делать это часто (то есть десятки раз в час), а n - из 1000, то у вас может быть проблема. Иначе я бы не потел, если вы удовлетворяете или превышаете спрос, а загрузка вашего ЦП / диска находится вне опасной зоны.