Включи мозг и извлекай, когда это имеет смысл

от автора

Сейчас уже сложно вспомнить тот момент, когда я впервые осознал, что выделять функции из больших кусков полезного кода, вообще-то, хорошая идея. То ли я получил это знание из “Совершенного кода”, то ли из “Чистого кода” — сложно вспомнить. Вообще-то, это не особенно важно. Мы все знаем, что должны разносить бизнес-логику по хорошо проименованным функциям. Самая длинная функция, которую я когда-либо видео в жизни была длиной в 5к строк. Я лично знаком с тем “программистом”, что написали тот код. Помню, как впервые встретил эту функцию. Не сложно предсказать, что моей первой реакцией было: “Какого чёрта!!! Кто произвёл на свет этот кусок дерьма???”
Да, представьте себе, этот “программист” до сих пор слоняется тут в офисе, где я сейчас работаю над текущими проектами. Не хочу углубляться в эту историю, но хочу упомянуть, что та функция длиной в 5к строк была ядром программы, размером примерно в 150к строк. Та программа в конце концов была загнана в угол, из-за того куска дерьма, который настолько сильно влиял на архитектуру приложения. В конце концов было принято решение о переписывании приложения с нуля.

Эта история иллюстрирует одну крайность проблемы размера функций, которая привела к плачевным последствиям. Другая крайность — отключить мозг и начать выделять повсюду классы с однострочными функциями внутри. Я не имею ввиду, что однострочные функции это плохо, я говорю о том, что не стоит забывать использовать мощности своего мозга. Вначале должно стоять рассмотрение проблемы.
Перед тем, как я продолжу исследовать проблему глубже, хотел бы отметить, что, вообще говоря, некоторое время назад произошла небольшая баталия между Дядей Бобом и Кристин Горман на данную тему. Дядя Боб представил технику, которую назвал “Extract till you drop”, которая вкратце означает — извлекай функции до тех пор пока, есть что извлекать. Кристин Горман сочла, что эта техника исключает использование мозга. Вдобавок, был пост Джона Сонмеза насчёт рефакторинга одной функции из .NET BCL (хотя, изначальной целью статьи было показать, что большая часть комментов — зло).

Давайте рассмотрим пример рефакторинга, проведённого Джоном. Он взял для рефакторинга следующий метод:

internal static void SplitDirectoryFile(      string path, out string directory, out string file) {     directory = null;     file = null;      // assumes a validated full path     if (path != null)     {         int length = path.Length;         int rootLength = GetRootLength(path);          // ignore a trailing slash         if (length > rootLength && EndsInDirectorySeparator(path))             length--;          // find the pivot index between end of string and root         for (int pivot = length - 1; pivot >= rootLength; pivot--)         {             if (IsDirectorySeparator(path[pivot]))             {                 directory = path.Substring(0, pivot);                 file = path.Substring(pivot + 1, length - pivot - 1);                 return;             }         }          // no pivot, return just the trimmed directory         directory = path.Substring(0, length);     }     return; } 

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

public class DirectoryFileSplitter {     private readonly string validatedFullPath;     private int length;     private int rootLength;     private bool pivotFound;     public string Directory { get; set; }     public string File { get; set; }      public DirectoryFileSplitter(string validatedFullPath)     {         this.validatedFullPath = validatedFullPath;         length = validatedFullPath.Length;         rootLength = GetRootLength(validatedFullPath);     }      public void Split()     {         if (validatedFullPath != null)         {             IgnoreTrailingSlash();              FindPivotIndexBetweenEndOfStringAndRoot();              if(!pivotFound)                 TrimDirectory();         }     }      private void TrimDirectory()     {         Directory = validatedFullPath.Substring(0, length);     }      private void FindPivotIndexBetweenEndOfStringAndRoot()     {         for (int pivot = length - 1; pivot >= rootLength; pivot--)         {             if (IsDirectorySeparator(validatedFullPath[pivot]))             {                 Directory = validatedFullPath.Substring(0, pivot);                 File = validatedFullPath.Substring(pivot + 1, length - pivot - 1);                 pivotFound = true;             }         }     }      private void IgnoreTrailingSlash()     {         if (length > rootLength && EndsInDirectorySeparator(validatedFullPath))             length--;     } } 

Вау, да? Не так уж и просто решить действительно ли рефакторинг помог сделать код более читабельным. Ощущение, что, вообще-то, его стало сложнее читать. Ранее была относительно небольшая функция с полезными комментариями, которая теперь была превращена в класс с четырьмя функциями внутри без комментариев. Я бы не сказал, что новый класс ллох и весь рефакторинг был плохой идеей, а программиста, проделавшего рефакторинг следует казнить. Совсем нет. Я не настолько кровожаден. Между этит двумя примерами кода есть несколько отличий. Рассмотрим эти отличия:

  1. Если вы пытаетесь достичь глубокого понимания того, что делает функция верхнего уровня, от функция стала более трудной для чтения, чем была изначально, поскольку теперь нужно проскакать по всем функциям и понять, что происходит в каждой из них. Напротив, первоначальный вариант легко можно быстро пробежать глазами.
  2. Если вы пытаетесь понять, что делает функция верхнего уровня концептуально, то отрефакторенный вариант проще для чтения, поскольку мы сразу видим, что функция концептуально делает внутри себя.
  3. Третье отличие, которое я вижу — стоимость поддержки. Что касается нашего конкретного примера, то я бы сказал, что стоимость поддержки отрефакторенной версии выше, чем исходной (вам как минимум надо провести рефакторинг). В целом, ответ на вопрос о том, какой вариант более дорог в поддержке лежит в плоскости требований. Эти требования диктуют нам, важно ли в данной ситуации следовать SRP (принцип единственной ответственности) или нет. Если эта функция может быть единожды написана и забыта до скончания времён, то нет никаких причин для того, чтобы тратить время на её рефакторинг. Напротив, если ожидается, что функциональность будет расти, тогда у вас есть все причины для того, чтобы отрефакторить функцию в отдельный класс.

Вдобавок хочу коснуться ситуации, когда вы случайно (или преднамеренно) натыкаетесь на подобную функцию в легаси-системе. Вы сразу рванёте извлекать класс с четырьмя функциями внутри? Мой совет — не делайте этого без каких-либо на то причин, даже если покрытие тестами вашей кодовой базы стремиться к 100%. Почему? Потому что тут нет никакого технического долга. Я говорю о серьёзном техническом долге, который является причиной страданий.

Таким образом, нет ничего плохого в технике “extract till you drop”. Вы просто должны держать в голове кое-какие соображения, по моему мнению.
Подытожив, хочу сказать, что вы никогда не должны совершать бессмысленных поступков. Вы должны сначала подумать, проанализировать, сделать заключение и только после этого действовать.

ссылка на оригинал статьи http://habrahabr.ru/post/263371/


Комментарии

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *