Статьи

5 принципов рефакторинга на примере

Январь 15, 2019

Этот пост содержит пять (в основном хорошо известных) принципов рефакторинга, применяемых при рефакторинге реального открытого кода ( Gradle Modules Plugin ).

контекст

Когда я работал над отдельной компиляцией module-info.java для плагина Gradle Modules (PR # 73 ), я заметил потенциал для некоторого рефакторинга. В результате я подал проблему № 79 и позже решил ее с помощью PR № 88 (еще не объединенного), где я произвел рефакторинг кода.

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

Принципы рефакторинга

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

Пять принципов, представленных здесь:

  1. Скрыть «как» с «что»
  2. Стремление к последовательности
  3. Избегайте глубокого вложения
  4. Отдельные проблемы (= принцип единой ответственности)
  5. Избегайте дублирования с умом (= Не повторяйте себя)

1. Скрыть «Как» с «Что»

Этот принцип является лишь частью / перефразировкой принципа чистого кода , сформулированного Робертом Мартином .

Для меня сокрытие «как» с «что» означает извлечение классов и методов всякий раз, когда:

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

Пример 1: updateRelativePath

Вот фрагмент из RunTaskMutator перед рефакторингом:

1
2
3
4
5
mainDistribution.contents(copySpec -> copySpec.filesMatching(patchModuleExtension.getJars(), action -> {
  RelativePath relativePath = action.getRelativePath().getParent().getParent()
      .append(true, "patchlibs", action.getName());
  action.setRelativePath(relativePath);
}));

и вот фрагмент после рефакторинга:

1
2
3
mainDistribution.contents(
    copySpec -> copySpec.filesMatching(patchModuleExtension.getJars(), this::updateRelativePath)
);

Подводя итог, мы:

  • спрятал, как обновить относительный путь
  • с тем, что мы делаем (= тот факт, что мы обновляем его).

Благодаря такому рефакторингу гораздо проще понять, что происходит с mainDistribution .

Для справки, содержание updateRelativePath доступно здесь .

Пример 2: buildAddReadsStream & buildAddOpensStream

Вот как выглядела TestTask класса TestTask до рефакторинга:

1
2
3
4
5
6
7
8
9
TestEngine.select(project).ifPresent(testEngine -> {
  args.addAll(List.of("--add-reads", moduleName + "=" + testEngine.moduleName));
 
  Set<File> testDirs = testSourceSet.getOutput().getClassesDirs().getFiles();
  getPackages(testDirs).forEach(p -> {
    args.add("--add-opens");
    args.add(String.format("%s/%s=%s", moduleName, p, testEngine.addOpens));
  });
});

и вот как это выглядит потом:

1
2
3
4
TestEngine.select(project).ifPresent(testEngine -> Stream.concat(
    buildAddReadsStream(testEngine),
    buildAddOpensStream(testEngine)
).forEach(jvmArgs::add));

Опять мы:

  • скрыл, как --add-reads значения параметров --add-reads и --add-opens
  • с тем, что мы делаем (= тот факт, что мы их указываем).

Для справки, содержимое buildAddReadsStream и buildAddOpensStream доступно здесь .

2. Стремление к последовательности

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

Например, сообщение Дональда Рааба в блоге о симметрии является отличным примером стремления к последовательности. Само собой разумеется, я полностью согласен с его заключением:

Большая система с хорошей симметрией становится легче для понимания, потому что вы можете обнаружить и ожидать повторяющиеся паттерны.

Дональд Рааб, Симметричная симпатия

В случае с плагином Gradle Modules это сводилось главным образом к извлечению базового класса AbstractModulePluginTask и унификации процедуры поиска задач и диспетчеризации.

Например, JavadocTask и TestTask до рефакторинга были:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
public class JavadocTask {
  public void configureJavaDoc(Project project) {
    Javadoc javadoc = (Javadoc) project.getTasks().findByName(JavaPlugin.JAVADOC_TASK_NAME);
    if (javadoc != null) {
      // ...
    }
  }
}
 
public class TestTask {
  public void configureTestJava(Project project, String moduleName) {
    Test testJava = (Test) project.getTasks().findByName(JavaPlugin.TEST_TASK_NAME);
    // ... (no null check)
  }
}

и после этого они:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
public class JavadocTask extends AbstractModulePluginTask {
  public void configureJavaDoc() {
    helper().findTask(JavaPlugin.JAVADOC_TASK_NAME, Javadoc.class)
        .ifPresent(this::configureJavaDoc);
  }
 
  private void configureJavaDoc(Javadoc javadoc) { /* ... */ }
}
 
public class TestTask extends AbstractModulePluginTask {
  public void configureTestJava() {
    helper().findTask(JavaPlugin.TEST_TASK_NAME, Test.class)
        .ifPresent(this::configureTestJava);
  }
 
  private void configureTestJava(Test testJava) { /* ... */ }
}

Для справки: JavaDocTask diff и TestTask diff .

3. Избегайте глубокого вложения

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

Как следствие, я реорганизовал следующий метод getPackages :

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
private static Set<String> getPackages(Collection<File> dirs) {
  Set<String> packages = new TreeSet<>();
  for (File dir : dirs) {
    if (dir.isDirectory()) {
      Path dirPath = dir.toPath();
      try (Stream<Path> entries = Files.walk(dirPath)) {
        entries.forEach(entry -> {
          if (entry.toFile().isFile()) {
            String path = entry.toString();
            if (isValidClassFileReference(path)) {
              Path relPath = dirPath.relativize(entry.getParent());
              packages.add(relPath.toString().replace(File.separatorChar, '.'));
            }
          }
        });
      } catch (IOException e) {
        throw new GradleException("Failed to scan " + dir, e);
      }
    }
  }
  return packages;
}

как ниже:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
private static Set<String> getPackages(Collection<File> dirs) {
  return dirs.stream()
      .map(File::toPath)
      .filter(Files::isDirectory)
      .flatMap(TestTask::buildRelativePathStream)
      .map(relPath -> relPath.toString().replace(File.separatorChar, '.'))
      .collect(Collectors.toCollection(TreeSet::new));
}
 
private static Stream<Path> buildRelativePathStream(Path dir) {
  try {
    return Files.walk(dir)
        .filter(Files::isRegularFile)
        .filter(path -> isValidClassFileReference(path.toString()))
        .map(path -> dir.relativize(path.getParent()));
  } catch (IOException e) {
    throw new GradleException("Failed to scan " + dir, e);
  }
}

Полный diff доступен здесь.

4. Отдельные проблемы

SRP ( принцип единой ответственности ) является хорошо известным принципом разработки программного обеспечения. Здесь мы видим его применение в извлечении StartScriptsMutator из RunTaskMutator .

Перед:

1
2
3
4
5
6
7
8
9
public class RunTaskMutator {
  // common fields
 
  public void configureRun() { /* ... */ }
 
  public void updateStartScriptsTask(String taskStartScriptsName) { /* ... */ }
 
  // 12 other methods (incl. 2 common methods)
}

После:

01
02
03
04
05
06
07
08
09
10
11
12
13
public class RunTaskMutator extends AbstractExecutionMutator {
 
  public void configureRun() { /* ... */ }
   
  // 2 other methods
}
 
public class StartScriptsMutator extends AbstractExecutionMutator {
 
  public void updateStartScriptsTask(String taskStartScriptsName) { /* ... */ }
 
  // 8 other methods
}

Благодаря извлечению StartScriptsMutator , гораздо легче понять области:

  • настройка задачи run как таковой,
  • настройка связанной задачи startScripts .

Для справки: коммит с вышеуказанным извлечением.

5. Избегайте дублирования с умом

СУХОЙ ( не повторяйся) — еще один известный принцип разработки программного обеспечения. Однако, по моему опыту, этот принцип иногда заходит слишком далеко, что приводит к тому, что код не дублируется, но и слишком сложен.

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

  • стоимость : время рефакторинга, итоговая сложность и т. д.
  • выгода : не дублирование (или, точнее, единый источник правды ).

Одним из таких примеров из плагина Gradle Modules (где коэффициент затрат-приближения близок к нулю, но все же положительный, на мой взгляд) является введение PatchModuleResolver .

Ниже приведен фрагмент кода перед рефакторингом, который состоит из:

  1. Метод PatchModuleExtension.configure .
  2. Место, где он используется ( TestTask ).
  3. Место, где его нельзя использовать ( RunTaskMutator ).
  4. Другое место, где его нельзя использовать ( JavadocTask ).
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
// 1. PatchModuleExtension
public List<String> configure(FileCollection classpath) {
  List<String> args = new ArrayList<>();
 
  config.forEach(patch -> {
        String[] split = patch.split("=");
 
        String asPath = classpath.filter(jar -> jar.getName().endsWith(split[1])).getAsPath();
 
        if (asPath.length() > 0) {
          args.add("--patch-module");
          args.add(split[0] + "=" + asPath);
        }
      }
  );
 
  return args;
}
 
// 2. TestTask
args.addAll(patchModuleExtension.configure(testJava.getClasspath()));
 
// 3. RunTaskMutator
patchModuleExtension.getConfig().forEach(patch -> {
      String[] split = patch.split("=");
      jvmArgs.add("--patch-module");
      jvmArgs.add(split[0] + "=" + PATCH_LIBS_PLACEHOLDER + "/" + split[1]);
    }
);
 
// 4. JavadocTask
patchModuleExtension.getConfig().forEach(patch -> {
      String[] split = patch.split("=");
 
      String asPath = javadoc.getClasspath().filter(jar -> jar.getName().endsWith(split[1])).getAsPath();
 
      if (asPath != null && asPath.length() > 0) {
        options.addStringOption("-patch-module", split[0] + "=" + asPath);
      }
    }
);

После введения PatchModuleResolver код выглядит следующим образом:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
// 1. PatchModuleExtension
public PatchModuleResolver resolve(FileCollection classpath) {
  return resolve(jarName -> classpath.filter(jar -> jar.getName().endsWith(jarName)).getAsPath());
}
 
public PatchModuleResolver resolve(UnaryOperator<String> jarNameResolver) {
  return new PatchModuleResolver(this, jarNameResolver);
}
 
// 2. TestTask
patchModuleExtension.resolve(testJava.getClasspath()).toArgumentStream().forEach(jvmArgs::add);
 
// 3. RunTaskMutator
patchModuleExtension.resolve(jarName -> PATCH_LIBS_PLACEHOLDER + "/" + jarName).toArgumentStream().forEach(jvmArgs::add);
 
// 4. JavadocTask
patchModuleExtension.resolve(javadoc.getClasspath()).toValueStream()
    .forEach(value -> options.addStringOption("-patch-module", value));

Благодаря рефакторингу, теперь есть только одно место ( PatchModuleResolver ), где мы разделяем записи PatchModuleExtension класса PatchModuleExtension .

Для справки: разности 1 , 2 , 3 , 4 .

Резюме

В этом посте я представил следующие пять принципов рефакторинга:

  1. Скрыть «как» с «что»
  2. Стремление к последовательности
  3. Избегайте глубокого вложения
  4. Отдельные проблемы
  5. Избегайте дублирования с умом

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

Опубликовано на Java Code Geeks с разрешения Томаша Линковски, партнера нашей программы JCG. См. Оригинальную статью здесь: 5 принципов рефакторинга на примере

Мнения, высказанные участниками Java Code Geeks, являются их собственными.

51
Share: