Рефакторинг: выделяй метод, когда это имеет смысл

Сейчас уже сложно вспомнить тот момент, когда я впервые осознал, что выделять функции из больших кусков полезного кода, вообще-то, хорошая идея. То ли я получил это знание из “Совершенного кода”, то ли из “Чистого кода” — сложно вспомнить. В целом, это не особенно важно. Мы все знаем, что должны разносить бизнес-логику по хорошо проименованным функциям. Самая длинная функция, которую я когда-либо видео в жизни была длиной в 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”. Вы просто должны держать в голове кое-какие соображения, по моему мнению.
Подытожив, хочу сказать, что вы никогда не должны совершать бессмысленных поступков. Вы должны сначала подумать, проанализировать, сделать заключение и только после этого действовать.