Утренний рефакторинг с Дженной Ортегой*
На относительно простом примере показываю как можно сделать программу «снова великой».
Исходный код отрефакторенной версии выложен на Github.
Задача
Допустим есть некий софт, созданный еще «при царе Горохе» неким гордым, но умным одиночкой, которого с тех пор никто не видел. Софт живой и с пользователями, которые приносят прибыль, поэтому его надо как‑то развивать и поддерживать.
В попытке расширить команду разработки, вы начинаете нанимать новых разработчиков, но раз за разом происходит одна и та же ситуация:
проработав месяц-два, нанятые программисты в ужасе убегают в закат.
Кто‑то при увольнении намекает на причины такого поступка, в диапазоне от «ваш проект попахивает» до надо «срочно все переписать». После примерно десятого убежавшего программиста, вы наконец начинаете задумываться, что возможно с проектом действительно что‑то не так и стоит провести этот самый «рефакторинг».
Так это обычно начинается.
Образец
В качестве образца для этой статьи был взят один интересный, но малоизвестный широкой публике проект JPC:
The fast x86 PC emulator in 100% pure Java
Самый настоящий эмулятор старого x86-компьютера, реализованный без всяких нативных частей — на чистой Java!
Как-то так это выглядит у автора (ов) в работе:
Проект старый, но и интересный:
Read more about JPC — since it’s launch at JavaOne 2007, JPC can now boot many more operating systems (including graphical linuxes) and it’s much faster.
Не очень большой (~6500 файлов с исходным кодом), но имеет стадию «внутренней генерации» — часть исходного кода создана не вручную, а путем запуска кодогенератора по метаданным, что довольно часто встречается у больших проектов с историей, причем на любом языке.
Еще к сожалению JPC немного заброшен, что для статьи только в плюс поскольку добавляет реалистичности — именно в таком состоянии чаще всего пребывают проекты, которые просят «привести в чувство».
Текущее состояние
После переезда проекта на Github, список коммитов выглядит следующим образом:
Никаких тестов в проекте нет, по всей видимости тестировалось все вручную с помощью молитвы, еще судя по исходному коду — далеко не весь функционал является рабочим, что также характерно для проектов в «пред‑рефакторинговом» состоянии.
Думаю теперь очевидно почему для этой статьи был выбран именно JPC — несмотря на редкость решаемой задачи, для рефакторинга это самый типичный клиент.
Собирается сей чудо-проект с помощью. Makefile, что для мира Java является дичью и извращением редкостью:
make applicationПримерно как собирать проект на QT с помощью Gradle или (еще лучше) — sbt.
Предложите как-нибудь коллегам и посмотрите на реакцию, некоторые точно перестанут с вами здороваться за руку.
Для сборки необходимо указать путь к JDK в переменной PATH, при этом сборка проходит успешно даже с последними версиями (автор использовал OpenJDK 21).
Готовое приложение JPCApplication.jar появится в корне проекта после завершения сборки, но на этом хорошие новости заканчиваются:
Собранное приложение отказывается запускаться, что мы исправим чуть ниже.
Стадия первая: новый скелет
Первым делом, как и в реальном боевом проекте, необходимо избавиться от любого «самопала», задействованного при сборке.
Причина, почему этот шаг критически важен на самом деле не так очевидна:
статические анализаторы — главный иструмент рефакторинга, крепко привязаны к структуре проекта и стандартным средствам сборки
Разумеется существуют варианты и с произвольной структурой проекта, но эффективность рефакторинга будет заметно ниже. Поэтому автор сделал стандартный (для своей практики) «финт ушами»:
перевел сборку проекта на Apache Maven, максимально широко поддерживаемый средствами анализа кода, CI-системами и средами разработки.
Реализовать такую миграцию в данном случае оказалось очень просто, поскольку JPC совсем не использует внешние библиотеки. Все что я сделал — раскидал ресурсы и исходный код в стандартную для Maven структуру каталогов:
Исходный код был перенесен из каталога src в src/main/java, ресурсы — в src/main/resources. Также был добавлен очень простой pom.xml, описывающий минимальные шаги сборки проекта:
4.0.0
com.x0x08.samples
jpc-refactored
1.0-SNAPSHOT
jar
UTF-8
17
17
org.apache.maven.plugins
maven-compiler-plugin
3.13.0
org.apache.maven.plugins
maven-jar-plugin
3.4.2
org.jpc.j2se.JPCApplication
Обратите внимание на куда более упрощенную генерацию манифеста:
org.jpc.j2se.JPCApplication
Указывается только стартовый класс, используемый для запуска приложения, а дата сборки и название будут проставляться автоматически самим Maven.
В оригинальной сборке происходит добавление еще и атрибута, отвечающего за параметры запуска «по-умолчанию»:
echo "Default-Args: -fda mem:resources/images/floppy.img -hda mem:resources/images/dosgames.img -boot fda" >> jpc.manifestЭто было убрано, поскольку точно такой же параметр запуска зашит еще и в код:
Наследие «былых времен», которое также достаточно часто встречается в устаревших проектах — во времена Java 1.5 и апплетов было модным использовать собственные атрибуты в манифесте.
Стадия вторая: удаление ненужного
Как в практически любом долгоживущем проекте, в JPC есть свои «внутренние утилиты» — отдельные программы, написанные для задач внутренней автоматизации.
Это та самая «грязная рабочая поверхность», которую не видит конечный пользователь.
При проведении рефакторинга, трогать внутренние утилиты стоит в последнюю очередь и в самом крайнем случае, поскольку правильность их работы проверять тяжело (ни тестов ни документации для внутренних утилит обычно нет в природе), зато они сильно влияют на общую работоспособность проекта.
В JPC внутренние утилиты реализованы в виде отдельных классов в пакете «tools» и нескольких шелл-скриптов в корне проекта.
И то и другое я просто не стал переносить в новую версию, также я убрал часть исходного кода эмулятора, отвечающего за отладку (пакет org.jpc.debugger) — по той же самой причине.
Что потребовало правок в классе org.jpc.emulator.PC.java:
был убран импорт класса org.jpc.debugger.LinearMemoryViewer, а используемая статичная функция translateLinearAddressToInt перенесена в класс PC.
Все эти действия позволили сократить кодовую базу проекта практически вдвое, что сильно упростило следующий шаг рефакторинга.
Стадия третья: критические проблемы
Наконец мы подошли непосредственно к самому рефакторингу, который я буду проводить с помощью среды разработки Intellij Idea.
Первый запуск анализатора дает следующий результат:
24 критических ошибки и ~ 37 тысяч предупреждений — не так уж плохо, по сравнению с тем что бывает на свете.
Смотрим глубже и видим, что все 24 ошибки — действительно самые критичные, поскольку из-за них проект может перестать собираться в самом ближайшем будущем:
Так что эти места стоит рефакторить в первую очередь, пока проект хотя-бы собирается из исходников.
Есть и хорошая новость:
Как видно из скриншота выше, большая часть критичных ошибок гнездится в классе JPCApplet, который используется для запуска приложения в режиме Java-апплета — ныне устаревшей технологии, когда-то работавшей с помощью плагина для браузера.
Поскольку плагин более официально не поддерживается — вся технология приказала долго жить и у обычных пользователей не встречается, так что класс можно удалить.
Но все несколько сложнее, поскольку еще есть вложенные классы, один из которых используется снаружи (org.jpc.j2se.JPCApplication):
JPCApplet.PlayPausePanel pp = new JPCApplet.PlayPausePanel(this);Я просто перенес этот класс по месту использования, что позволило наконец удалить JPCApplet из проекта целиком.
Получилось минус 13 критических ошибок.
Еще один источник проблем — класс LinkBorder также можно удалить, поскольку он использовался лишь из удаленного JPCApplet.
Что дало еще минус три критических ошибки.
Дальше смотрим класс org.jpc.emulator.peripheral.Mixer, который забит предупреждениями от анализатора буквально через каждую строчку, однако вносить массовые правки пока не стоит — «всемогущая» Idea временами ошибается и это именно такой случай.
Ограничимся лишь одним методом:
private void ShowVolume(String name,FloatRef vol0,FloatRef vol1) {
System.out.printf("%-8s %3.0f:%-3.0f %+3.2f:%-+3.2f \n",new Object[] {name,
new Float(vol0.value*100),new Float(vol1.value*100),
new Float(20*Math.log(vol0.value)/Math.log(10.0f)),new Float(20*Math.log(vol1.value)/Math.log(10.0f))}
);
}Анализатор ругается (в первую очередь) на конструктор new Float(), поскольку его прямое использование объявлено устаревшим, а в новых версиях Java стоит использовать Float.valueOf() в качестве замены.
Но как только вы замените конструктор, анализатор подскажет еще несколько оптимизаций, так что конечный вариант будет достаточно сильно отличаться:
System.out.printf("%-8s %3.0f:%-3.0f %+3.2f:%-+3.2f \n",
name,
vol0.value * 100, vol1.value * 100,
20*Math.log(vol0.value)/Math.log(10.0f),
20*Math.log(vol1.value)/Math.log(10.0f));Следующий класс для изучения org.jpc.j2se.PCMonitorFrame, где анализатор ругается на два места с критическими ошибками.
Первое место характерно для устаревших проектов и встретится еще не раз:
catch (AccessControlException e)
{
LOGGING.log(Level.WARNING, "Not able to add some components to frame.", e);
}Дело в том что класс ошибки, которая тут обрабатывается объявлен устаревшим в новых версиях:
'java. security. AccessControlException' is deprecated since version 17 and marked for removal
Поскольку исключение AccessControlException в новых версиях не выбрасывается, весь блок try-catch можно спокойно удалить.
Следующее место, код в этом же классе:
if (runner.isAlive())
{
try
{
runner.stop();
}
catch (SecurityException e) {}
}Ругается анализатор на уникальный метод stop(), который был отмечен как устаревший еще до того как я начал писать на Java:
'stop ()' is deprecated since version 1.2 and marked for removal
Примерно до версии 1.8 использование данного метода еще можно было как‑то оправдать наличием устаревших библиотек, в нынешних реалиях этот метод — просто еще один способ «выстрелить себе в ногу»:
Stopping a thread causes it to unlock all the monitors that it has locked.
Так что в коде использование этого метода точно стоит заменить на стандартный .interrupt () :
if (runner.isAlive())
{
runner.interrupt();
}Блок try-catch также можно спокойно убрать, поскольку SecurityException не выбрасывается в новых версиях Java при попытке остановки нити.
На этом все критические проблемы в проекте решены и получен минимальный практический смысл от всей затеи:
убраны места, которые могут сломать сборку проекта в новых версиях Java
Стадия четвертая: ошибки выполнения
Пришло время наконец попробовать запустить нашего «франкенштейна».
Сборка разумеется завершится успешно (не зря же старались), но при запуске будет выбрасываться все та же ошибка поиска ресурсов:
В оригинальной версии JPC, часть ресурсов (например образы биоса) загружались только из jar‑файла, часть (образы дисков) — только снаружи, из каталога resources, при этом каталог с ресурсами был общим.
Сию дичь необходимо пресечь и сделать в более адекватном стиле, например как это реализовано в движке знаменитого Quake:
сначала ищем внешний файл, если не найден — ищем в ресурсах, если не найден в ресурсах — падаем с ошибкой
За чтение образа BIOS отвечает вот такой метод в классе org.jpc.emulator.motherboard.Bios:
private static final byte[] getBiosData(String image) throws IOException {
InputStream in = Bios.class.getResourceAsStream(image);
if (in == null) {
throw new IOException("resource not found: " + image);
}
try {
ByteArrayOutputStream bout = new ByteArrayOutputStream();
while (true) {
int ch = in.read();
if (ch < 0) {
break;
}
bout.write((byte) ch);
}
return bout.toByteArray();
} finally {
try {
in.close();
} catch (IOException e) {
}
}
}
}В принципе за такую реализацию уже можно начинать бить, спасает лишь факт, что столь идиотсткое побайтовое чтение работает исключительно с ресурсами, которые уже находятся в памяти.
Конечный вариант после всех чисток выглядит так:
private static byte[] getBiosData(String image) throws IOException {
File f = new File(image);
if (f.exists() && f.isFile() && f.canRead())
return Files.readAllBytes(f.toPath());
f = new File("resources",image);
if (f.exists() && f.isFile() && f.canRead())
return Files.readAllBytes(f.toPath());
final URL u = Bios.class.getResource(image);
if (u == null)
throw new IOException("resource (bios) not found: %s".formatted(image));
try (InputStream in = u.openStream()) {
return in.readAllBytes();
}
}Логика переделана на возможности современной Java 17, поэтому кода стало сильно меньше, также были добавлены проверки на наличие ресурса:
по полному пути,
по частичному (предполагается что файл находится в каталоге resources),
поиск внутри jar приложения.
Но при следующей попытке запуска получаем еще одно исключение, уже в другом месте:
Причиной является искусственная проверка:
if (!(cl instanceof URLClassLoader))
throw new IllegalStateException(); Когда-то давно системный загрузчик классов действительно наследовался от URLClassLoader, так что проверка бы отработала.
Несмотря на то, что подобные искусственные проверки служат вообщем‑то хорошей цели раннего обнаружения проблем, временами разработчики перебарщивают и пытаются контролировать то что контролю не поддается.
Однако одним лишь удалением проверки дело не ограничилось — необходимо почистить еще один метод, реализующий «закат солнца вручную»:
private static final Iterator getResources(String directory)
{
ClassLoader context = Thread.currentThread().getContextClassLoader();
List resources = new ArrayList();
ClassLoader cl = JPCApplication.class.getClassLoader();
if (!(cl instanceof URLClassLoader))
throw new IllegalStateException();
URL[] urls = ((URLClassLoader) cl).getURLs();
int slash = directory.lastIndexOf("/");
String dir = directory.substring(0, slash + 1);
for (int i=0; i= 0)
thisDir = name.substring(0, slash + 1);
if (!dir.equals(thisDir))
continue;
resources.add(name);
}
jarStream.close();
}
catch (IOException e) { e.printStackTrace();}
}
InputStream stream = context.getResourceAsStream(directory);
try
{
if (stream != null)
{
Reader r = new InputStreamReader(stream);
StringBuilder sb = new StringBuilder();
char[] buffer = new char[1024];
try
{
while (true)
{
int length = r.read(buffer);
if (length < 0)
{
break;
}
sb.append(buffer, 0, length);
}
} finally
{
r.close();
}
for (String s : sb.toString().split("\n"))
{
if (context.getResource(directory + s) != null)
{
resources.add(s);
}
}
}
}
catch (IOException e)
{
LOGGING.log(Level.INFO, "Exception reading images directory stream", e);
}
return resources.iterator();
}
Тут происходит поиск доступных образов дисков путем последовательного перебора всех файлов внутри .jar с приложением.
С учетом того что .class файлов внутри ~6500 — такое решение мягко говоря «не оптимально».
Вообще говоря любой поиск ресурсов через перебор во время работы приложения является медленным, это и есть основная причина медленного запуска любого приложения на (например) Spring Boot.
Поскольку в проекте используется очень небольшое количество образов диска и нет вариантов по резкому увеличению их количества, я просто зашил названия в код:
private static final String[] IMAGES = {
"images/dosgames.img",
"images/floppy.img",
"images/linux.img",
"images/odin070.img",
};
private static Iterator getResources(String directory)
{
final List resources = new ArrayList
(Arrays.stream(IMAGES).toList());
final File f = new File(directory);
if (!f.exists() || !f.isDirectory()) {
return resources.iterator();
}
final File[] files = f.listFiles();
if (files == null) {
return resources.iterator();
}
for (File ff: files) {
resources.add(directory + ff.getName());
}
return resources.iterator();
} Метод getResources() используется для отображения списка доступных образов дисков через меню приложения, все внутренние образы (зашитые в.jar) добавляются в этот список автоматически.
После столь примитивной правки, приложение стало запускаться визуально быстрее даже на мощном современном ноутбуке, так что не стоит недооценивать силу простых решений ;)
Хотя всех правок выше оказалось недостаточно, следующая остановка — класс org.jpc.support.ArrayBackedSeekableIODevice, который (внезапно) играет ключевую роль в проекте.
Метод configure() отвечает непосредственно за загрузку образов дисков и дискет:
public void configure(String spec) throws IOException
{
resource = spec;
imageOffset = 0;
InputStream in = ArrayBackedSeekableIODevice.class.getClassLoader().getResourceAsStream(resource);
if (in == null) {
LOGGING.log(Level.SEVERE, "resource not found: {0}", resource);
throw new IOException("resource not found: " + resource);
}
try {
byte[] buffer = new byte[1024];
ExposedByteArrayOutputStream bout = new ExposedByteArrayOutputStream(32*1024);
while (true) {
int read = in.read(buffer);
if (read < 0)
break;
bout.write(buffer, 0, read);
}
imageData = bout.getBuffer();
length = bout.getPosition();
} catch (IOException e) {
LOGGING.log(Level.SEVERE, "could not load file", e);
throw e;
} finally {
try {
in.close();
} catch (IOException e) {
}
}
}
Переделываем с учетом современных реалий:
public void configure(String spec) throws IOException {
resource = spec;
imageOffset = 0;
File f = new File(spec);
if (f.exists() && f.isFile() && f.canRead()) {
imageData = Files.readAllBytes(f.toPath());
length = imageData.length;
return;
}
f = new File("resources",spec);
if (f.exists() && f.isFile() && f.canRead()) {
imageData = Files.readAllBytes(f.toPath());
length = imageData.length;
return;
}
final URL u = ArrayBackedSeekableIODevice.class.getResource(spec);
if (u == null)
throw new IOException("resource (image) not found: %s"
.formatted(spec));
try (InputStream in = u.openStream()) {
imageData = in.readAllBytes();
length = imageData.length;
}
}Тут тоже три последовательные проверки на поиск образа диска и использование современного API для минимизации кодовой базы.
Эта доработка оказалась финальной и наконец можно успешно запустить эмулятор:
java -jar target/jpc-refactored-1.0-SNAPSHOT.jar -hda src/main/resources/images/linux.img Будет запущен Qemu Linux Test Distribution:
Или так:
java -jar target/jpc-refactored-1.0-SNAPSHOT.jar -fda mem:images/odin070.img запустится FreeDOS ODIN:
На этой стадии была проведена самая настоящая «коммерческая оптимизация» — доведен до ума функционал актуальный конечным пользователям.
Это уже не стандартные сказки про «технический долг» и «плохую архитектуру», а вполне себе осязаемый результат, который можно потрогать.
Так что вас за такое-то скотство, проведенное с рабочим проектом уже точно не уволят ;)
Стадия пятая: массовые правки
Все описанное выше — обязательные базовые части, без которых рефакторинг вообще не может состояться как согласованный с бизнесом и оплаченный процесс.
Но можно зайти дальше — в действительно рисковую зону, где ваши действия могут иметь не всегда предсказуемые последствия:
массовые и сквозные правки исходного кода, во всем проекте целиком
Рабочая область выглядит как-то так:
Собственно все «желтенькое» на скриншоте ниже — места для рефакторинга, заботливо подсказанные средой разработки:
К сожалению на практике все несколько сложнее чем подсказывает Idea и просто нажимать «Alt + Shift + Enter» на каждую подсказку не стоит:
Все потому, что в проекте активно используется Reflection API для загрузки и обращения к методам класса необычными способами:
try {
Method valueOf = type.getMethod("valueOf", String.class);
return valueOf.invoke(type, value);
} catch (NoSuchMethodException e) {
System.err.println(type + " :No suitable method");
}Что сводит анализаторы исходного кода с ума, поэтому примерно половина методов в проекте десктоп-приложения, не использующего никакие IoC-контейнеры отмечено как неиспользуемые:
Скотство?
Конечно скотство, но и в реальных больших и старых проектах такое тоже будет в обязательном порядке — когда‑то использование Reflection API считалось модным и молодежным явлением, убрать которое под капот смогли только те самые IoC‑контейнеры вроде Spring и ORM вроде Hibernate.
Следующим примером кода, нуждающегося в массовой зачистке является использование анонимных классов:
Лямбды появились еще в Java 8 и с тех пор уже нет никакого здравого смысла их не использовать — они здорово сокращают объем кода:
В любом legacy-проекте, особенно если это приложение для десктопа такого будет очень и очень много:
Следующий повод для массовых правок — прямой результат ручной разработки, без использования средств проверки и анализа кода:
Хороший пример, подсказанный анализатором:
Разумеется это не является критической проблемой, поскольку эти модификаторы ничего не делают, но таких мест очень много и в сумме они дают ненужное увеличение объема кодовой базы.
Следующие две проблемы — также частые гости устаревших проектов:
Точно также как и с ненужными модификаторами в интерфейсе, всего лишь занимают место и увеличивают объем кода.
Хотя пример выше это совсем уж старый код, поскольку метод Arrays.asList () появился еще в Java 7 — былинные времена далекого прошлого, как можно было его сохранить до сих пор — загадка.
Эпилог
Если вы никогда не видели JPC то стоит посмотреть, поскольку он в свое время несколько расширил мнение о возможности Java, в первую очередь в плане производительности — тема о которой много и сильно шутили еще 10 лет назад.
Ну, а если перед вами стоит задача провести подобный рефакторинг — стадии с первой по четвертую фактически являются руководством к действию.
Массовые правки я бы с ходу делать не рекомендовал — очень уж высокие риски, что что‑то пойдет не так.
Еще в реальном проекте процесс рефакторинга скорее всего затянется, поэтому вам придется делать промежуточные срезы и синхронизировать ваш рефакторинг с обычной разработкой, о чем стоит помнить до начала всего действа.
P.S.
Это переработанная и обновленная версия статьи, оригинал которой доступен в нашем блоге.
0×08 Software
Мы небольшая команда ветеранов ИТ‑индустрии, создаем и дорабатываем самое разнообразное программное обеспечение, наш софт автоматизирует бизнес‑процессы на трех континентах, в самых разных отраслях и условиях.
Оживляем давно умершее, чиним никогда не работавшее и создаем невозможное — затем рассказываем об этом в своих статьях.
