Статьи

Рефакторинг с помощью циклов и конвейеров сбора: Часть II, Вложенные циклы

Ноябрь 6, 2017

ПРИМЕЧАНИЕ РЕДАКТОРА: «Рефакторинг с помощью циклов и конвейеров сбора» — это серия из четырех статей Мартина Фаулера. Вы можете прочитать первую часть здесь .

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

Вложенная петля — читатели книг

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

интерфейс DataService… 

  Map<String, Collection<String>> getBooksReadOn(Date date);

для этого примера я переключусь на Java, потому что надоел имена методов с заглавной первой буквой

Вот петля

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  for (Map.Entry<String, Collection<String>> e : data.entrySet()) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey())) {
         result.add(e.getKey());
      }
    }
  }
  return result;
}

Я буду использовать мой первый обычный шаг, который заключается в применении Extract Variable к коллекции циклов

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet();
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey())) {
         result.add(e.getKey());
      }
    }
  }
  return result;
}

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

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

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey()))) {
        result.add(e.getKey());
      }
    }
  }
  return result;
}

В библиотеке потоков Java конвейер должен заканчиваться терминалом (таким как коллектор)

Другое предложение сложнее переместить, поскольку оно относится к переменной внутреннего цикла. Этот пункт проверяет, содержит ли значение записи карты какую-либо книгу, которая также есть в списке книг в параметре метода. Я могу сделать это с помощью пересечения множества. Базовые классы Java не включают метод пересечения множеств. Я могу получить с помощью одной из распространенных надстроек, ориентированных на коллекцию Java, таких как Guava или Apache Commons . Поскольку это простой педагогический пример, я напишу грубую реализацию.

Класс Utils … 

  public static <T> Set<T> intersection (Collection<T> a, Collection<T> b) {
    Set<T> result = new HashSet<T>(a);
    result.retainAll(b);
    return result;
  }

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

Теперь я могу переместить предложение из цикла в конвейер.

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) ) {
        result.add(e.getKey());
      }
    }
  }

  return result;
}

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

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  result = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .map(e -> e.getKey())
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      result.add(e.getKey());
    }
  }

  return result;
}

Тогда я могу использовать Инлайн Temp на result.

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  return data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .map(e -> e.getKey())
          .collect(Collectors.toSet());
  return result;
 }

Глядя на это использование пересечения, я нахожу, что это довольно сложно, я должен выяснить, что он делает, когда я читаю его — что означает, что я должен извлечь его.

Класс Utils … 

  public static <T> boolean hasIntersection(Collection<T> a, Collection<T> b) {
    return !intersection(a,b).isEmpty();
  }

Класс Сервис … 

  public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
    Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
    return data.entrySet().stream()
            .filter(e -> readers.contains(e.getKey()))
            .filter(e -> Utils.hasIntersection(e.getValue(), books))
            .map(e -> e.getKey())
            .collect(Collectors.toSet());
   }

Объектно-ориентированный подход находится в невыгодном положении, когда вам нужно сделать что-то подобное. Переключение между статическими служебными методами и обычными методами на объектах неудобно. В некоторых языках у меня есть способ сделать его читаемым как метод в поточном классе, но у меня нет такой опции в Java.

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

35
Share: