Рефакторинг вглубь

Рефакторинг – это “это контролируемая техника совершенствования структуры существующего кода” [Фаулер]. Сейчас уже написано так много всего о запахах кода и приёмах рефакторинга в микромасштабе (есть, например, книги и целые сайты). А я хочу рассмотреть ситуацию крупным планом и обсудить, как именно и в каком порядке следует применять эти приёмы. В частности, берусь утверждать, что рефакторинг лучше всего выполнять наизнанку, то есть, начинать от границы с внешним API, а далее прорабатывать код вглубь, переходя к классам, методам, алгоритмам, типам, тестам или именам переменных.

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

Сквозной пример

Вдохновившись примером Штефана, который он продемонстрировал на воркшопе «Refactoring Rust» на конференции EuroRust 2023, предложу такой пример. Допустим, у нас есть очень хитросплетённая функция parse_magic_key, которая перебирает файлы в каталоге, находит все файлы в формате JSON, разбирает файлы в поисках конкретных ключей и возвращает все соответствующие значения. Может быть, однажды эту функцию написали для прототипа, либо она появилась в ходе очередного хакатона, а может быть, она сама росла и доросла до сотни строк. Ниже приведу сокращённую версию этой умопомрачительной функции – примерно так она могла бы выглядеть:

use std::fs; use std::fs::File; use std::io::{BufRead, BufReader};  /// Разбирает синтаксис и возвращает поля `magic_key` из всех файлов JSON, расположенных в заданном каталоге. pub fn parse_magic_key(path: &str) -> Vec<String> {     let mut results = Vec::new();     for path in fs::read_dir(path).unwrap() {         let path = path.unwrap().path();         if path.extension().unwrap() == "json" {             // Ниже вашему вниманию предлагаются сотни строк с переусложнённой/уродливой/хрупкой логикой синтаксического разбора, которые здесь упрощены              // всего до нескольких строк.             let file = BufReader::new(File::open(path).unwrap());             for line in file.lines() {                 let line = line.unwrap();                 let parts: Vec<&str> = line.split(":").collect();                 if parts.len() == 2 && parts.get(0).unwrap().contains("magic_key") {                     results.push((*parts.get(1).unwrap()).trim().to_string());                 }             }         }     }      results }  #[cfg(test)] mod tests {     use std::fs;     use tempdir::TempDir;     use crate::parser::parse_magic_key;      #[test]     fn can_parse_dir() {         let tmp_dir = TempDir::new("").unwrap();         let dir = tmp_dir.as_ref();         fs::write(dir.join("positive.json"), r#"             {               "magic_key": "1"             }"#).unwrap();         fs::write(dir.join("negative.json"), r#"             {               "foo": "2"             }"#).unwrap();         fs::write(dir.join("negative.not-json"), r#"             {               "magic_key": "3"             }"#).unwrap();          assert_eq!(parse_magic_key(dir.to_str().unwrap()), vec![r#""1""#]);     } } 

Разумеется, в этом коде больше недостатков, чем достоинств. Например:

• Наивная/кривая/хрупкая логика синтаксического разбора
• Отсутствует вменяемая обработка ошибок
• Сложно понять, каково ожидаемое поведение этой функции
• Запутанная логика обхода каталогов и синтаксического разбора
• Жёстко запрограммированные допущения формата JSON, которые поэтому плохо поддаются расширению
• Эту функцию сложно развивать
• Негибкий и неясный API (напр., что именно означает path: &str?)
• Ёмкостная сложность O(|result|)

Допустим, мы хотим отрефакторить этот код. Естественно, первым делом возникает вопрос: с чего начать? Может быть, первым делом взяться за самые мелкие и очевидные детали, например, подыскать более подходящие имена переменных и методов, разбить метод на несколько более мелких? Может быть, для начала поправлю логику синтаксического разбора, например, задействую serde_json вместо этого неисправного ущербного парсера? Либо, всё-таки, начать с API — например, типов или сигнатуры метода?

Немного присмотревшись, сразу различаем здесь два пути, которыми можем пойти при рефакторинге: вглубь и вовне.

Вглубь или вовне?

Когда рефакторинг идёт вглубь, мы начинаем работу с деталей реализации. Мы исправляем логику синтаксического разбора (и добавляем тесты), подыскиваем более подходящие имена переменных. Далее, возможно, добавим поддержку других форматов кроме JSON. Наконец, когда внутренняя часть нас устроит, исправим API.

Подождите, а зачем же нам исправлять API? И откуда нам знать, какего исправить? Что ж, берусь утверждать, что любое внятное начинание, связанное с рефакторингом, должно начинаться с гипотезы и постановки целей проектирования: мы выполняем рефакторинг потому, что ____. Кстати, поскольку база кода уродлива, пользы в такой цели почти наверняка нет. Скорее, мы рефакторим базу кода по той причине, что сама её структура или детали реализации более несовместимы с поставленными требованиями и с теми случаями, в которых её приходится применять. Например:

• Нам нужен более гибкий API, чтобы переиспользовать модуль в другом контексте или приложении.
  В рассматриваемом фрагменте кода: поскольку мы передаём Path, а это путь к каталогу, мы не можем лишь выборочно разбирать файлы JSON; или все, или ни одного.
• База кода спроектирована так, что неизбежно ограничивает производительность, и теперь для нас это уже неприемлемо.
  В рассматриваемом фрагменте кода: поскольку весь результат должен умещаться в главной памяти, мы не можем разбирать большие объёмы данных.

Часто такие изменения в требованиях непосредственно приводят к изменениям в API модуля или функции. Так и зародилась идея рефакторинга вглубь: сначала меняем API, а затем переходим ко всё более мелким его деталям, пока реализация не станет нас устраивать. Эту идею описывали и ранее, правда, на более «корпоративном» языке и применительно к энтерпрайзу. Она называлась паттерн «душитель».

Рефакторинг вглубь

Шаг 0: Понять старый API и реализацию

За исключением случаев, когда вы ничем не стеснены и начинаете разработку с нуля, вам придётся поддерживать все предусмотренные, а зачастую даже непредусмотренные поведения старого API и всей реализации. Затем — держим в уме забор Честертона — перечисляем все те аспекты старого API, без которых можно обойтись. Выявив, какие ограничения или паттерны вызовов более не используются и значит не нужны, зачастую удаётся значительно упростить имеющиеся системы.

Шаг 1: Проектируем новый API

Когда мы проанализировали старый API и поняли, что из него требуется сохранить, что можно выбросить, а что нужно добавить или изменить, приходит время расписать новый API. В нашем примере новый API может выглядеть примерно так:

pub fn parse(files: impl IntoIterator<Item=PathBuf>)     -> impl Iterator<Item=String> {   todo!() }

Мы изменили входной параметр: теперь это не путь к каталогу, а итератор, перебирающий подаваемые на вход файлы. Теперь вызывающая сторона может контролировать, какие именно файлы следует разбирать. Во-вторых, функция возвращает итератор, перебирающий результаты. В таком случае вызывающей стороне становится проще потреблять результаты сплошным потоком в рамках ограниченной памяти. Конечно же, такая структура API не подойдёт на все случаи жизни, но в рамках нашего упражнения давайте допустим: именно это нам и нужно. (Чтобы не переусложнять пример, я решил не рассматривать здесь обработку ошибок).

Шаг 2: Реализуем новый API средствами старого кода

На следующем этапе постараемся реализовать новый API, воспользовавшись старым кодом и внеся в него такой минимум изменений, какой только возможно. Это важно: на данном этапе мы не стремимся написать идеальную новую реализацию, а хотим повторно использовать уже имеющуюся и приспособить её к новому API. Вот, например, как может выглядеть такая адаптация:

/// Разбирает и возвращает поля `magic_key` из всех файлов JSON, расположенных в заданном каталоге. /// TODO: обработка ошибок pub fn parse(files: impl IntoIterator<Item=PathBuf>)     -> impl Iterator<Item=String> {     // TODO: Разбирать результаты поступательно, а не собирать их все в один вектор      let mut results = vec![];     for path in files {         if let Ok(file) = File::open(&path) {             let extension = path.extension().map(|p| p.to_str()).flatten().expect("Failed to determine file extension");             let file_results = match extension {                 "json" => parse_json(file),                 // TODO: поддержка YAML                 // TODO: поддержка XML                 _ => {                     println!("Unknown extension: {extension}");                     vec![]                 }             };              results.extend(file_results);         }     }      results.into_iter() }  fn parse_json(read: impl Read) -> Vec<String> {     let reader = BufReader::new(read);     let mut results = Vec::new();     for line in reader.lines() {         let line = line.unwrap();         let parts: Vec<&str> = line.split(":").collect();         // TODO: использовать потоковый парсер JSON          // TODO: добиться, чтобы логика парсинга стала менее хрупкой         if parts.len() == 2 && parts.get(0).unwrap().contains("magic_key") {             results.push((*parts.get(1).unwrap()).trim().to_string());         }     }      results }  /// Разбирает и возвращает поля `magic_key` из всех файлов JSON, расположенных в заданном каталоге. pub fn parse_magic_key(path: &str) -> Vec<String> {     let mut results = Vec::new();     for path in fs::read_dir(path).unwrap() {         let path = path.unwrap().path();         if path.extension().unwrap() == "json" {             let file = File::open(path).unwrap();             results.append(&mut parse_json(file));         }     }      results }

Вот что мы сделали:

• Извлекли код синтаксического разбора в новый приватный метод parse_json
• Переписали исходный метод parse_magic_key с применением извлечённого метода parse_json
• Реализовали новый метод parse, воспользовавшись извлечённым методом parse_json
• Добавили TODO везде, где мы немного поленились и просто отметили те вещи, которые понадобится отрефакторить или оптимизировать позже

Обратите внимание: API, поведение и тесты исходного метода parse_magic_key остались неизменными.

Шаг 3: Написать тесты для нового API

Далее можно написать несколько тестов для нового API, например:

    #[test]     fn can_parse_json() {         assert_eq!(parse_json(r#"             {               "magic_key": "1"             }"#.as_bytes()), vec![r#""1""#]);          assert!(parse_json(r#"             {               "foo": "1"             }"#.as_bytes()).is_empty());     }      #[test]     fn can_parse_files() {         let tmp_dir = TempDir::new("").unwrap();         let dir = tmp_dir.as_ref();         fs::write(dir.join("positive.json"), r#"             {               "magic_key": "1"             }"#).unwrap();         fs::write(dir.join("negative.json"), r#"             {               "foo": "2"             }"#).unwrap();         fs::write(dir.join("negative.not-json"), r#"             {               "magic_key": "3"             }"#).unwrap();          let files = vec![dir.join("positive.json"), dir.join("negative.json"), dir.join("negative.not-json")];         let results: Vec<String> = parse(files).collect();         assert_eq!(results, vec![r#""1""#]);     }

Шаг 4: Исправляем внутренности

Наконец, можно поправить все те TODO, которые мы успели собрать по пути. Например, гарантировать, что логика синтаксического разбора корректна, обеспечить поддержку других типов файлов. Хочу отдельно отметить, что в первичной реализации шага 2 остаются актуальными те же требования к памяти, что и в исходной реализации. Правда, ключевой момент здесь заключается в том, что такой API рассчитан на отложенную дополнительную эффективность.

Обратите внимание, что шаги 1–2–3–4 можно выполнять в рамках независимых коммитов или даже запросов на слияние. Большинство TODO, перечисленных в шаге 4, можно выполнять параллельно. Их можно рецензировать независимо, и после каждого этапа сборка становится «зелёной». По-прежнему действуют все методы синтаксического разбора, вызванные ранее. Такой подход к изменению кода можно назвать «адиабатическим».

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

Но зачем

Мы рассмотрели, как устроен рефакторинг вглубь, но пока не обсудили, зачем это делать. Существуют «внутренние» и «внешние» причины, по которым, как подсказывает мой опыт, рефакторинг вглубь обычно оказывается эффективнее, чем рефакторинг вовне.

«Внутренняя» природа нового API, спроектированного нами на этапе 1, позволяет задать целевые показатели для дальнейшего рефакторинга. Я имею в виду, мы можем обоснованно считать рефакторинг завершённым, когда найдём разумную реализацию для нашего API. В конце концов, в новый API должны быть заложены все уже существующие плюс предполагаемые новые паттерны применения (например, постепенный парсинг) и ограничения (например, лимит памяти) для нашей библиотеки или приложения.

Например, поскольку нам поставлена задача создать, а не , можно нацелиться на такую реализацию, в которой используется ограниченный объём памяти. Таким образом, наша реализация приведёт нас к потоковому решению или LL-парсеру, который обычно читает от начала к концу. Кроме того, у нас ведь сохранились тесты (а возможно, и бенчмарки) по «старому» поведению – ими тоже можно воспользоваться, чтобы избежать случаев регрессии.

«Внешний» аспект здесь ещё интереснее. Если мы сначала расписываем новый API, мы (отчасти) отвязываем рефакторинговый труд от производительного труда тех разработчиков, которые хотят использовать новый API. Как только новый API добавлен в основную ветку, ваши коллеги по команде могут начинать с ним экспериментировать. В оптимальном случае они будут самостоятельно добиваться прогресса, работая каждый над своим участком кодовой базы (пользуясь при этом новым API). Чаще они смогут давать обратную связь по API, так как что-то точно пойдёт не так. Это и есть настоящий аджайл без аджайл-коуча.

Верно и обратное: почти наверняка вам приходилось сталкиваться с недостатками описанного подхода. «Новый API пока сырой, потому что рефакторинг начали со внутренних деталей алгоритма и начали перекомпоновывать базу данных. Мы дадим вам знать, когда будет готово, ещё неделя-две [Точно!]».

Заключение

Надеюсь, я убедил вас, что следующий проект по рефакторингу стоит начать именно «вглубь». Сначала напишите API, а затем занимайтесь всем прочим. Как и всегда при программной инженерии, такой метод – не панацея, но попробовать стоит.

P.S. Обращаем ваше внимание на то, что у нас на сайте проходит распродажа.