Refactoring: highlight a method when it makes sense

Now it’s hard to remember the moment when I first realized that extracting functions from large chunks of useful code is actually a good idea. Either I got this knowledge from the “Perfect Code” , or from the “Clean Code” - it's hard to remember. In general, this is not particularly important. We all know that we need to span business logic into well-named functions. The longest function that I have ever video in life was 5k lines. I am personally familiar with the “programmer” who wrote that code. I remember the first time I met this feature. It is not difficult to predict that my first reaction was: “What the hell !!! Who produced this piece of shit ??? ”
Yes, imagine this “programmer” still loitering here in the office where I am currently working on current projects. I do not want to delve into this story, but I want to mention that that function of 5k lines in length was the core of the program, about 150k lines in size. The development of the program ultimately came to a standstill, due to the terrible function that had an extremely negative impact on the architecture of the application. In the end, it was decided to rewrite the application from scratch.

This story illustrates one extreme of the function size problem, which led to disastrous consequences. The other extreme is to turn off the brain and start allocating classes with single-line functions inside. I do not mean that such functions are bad, I’m saying that you should not forget to use the power of your brain. The problem should be ananized first.
Before I continue to investigate the problem more deeply, I would like to note that, generally speaking, some time ago there was a small battle between Uncle Bob and Christine Gorman on this subject. Uncle Bob introduced a technique called “Extract till you drop,” which in short means “extract functions until there is something to extract.” Christine Gorman considered this technique to preclude the use of the brain. In addition, there was a post by John Sonmez about refactoring one function from the .NET BCL (although the original purpose of the article was to show that most of the comments are evil).

Let's look at an example of refactoring by John. He took the following method as an example:

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;
}

In order to make this code easier to read, John created a new class by placing the refactored source method in it. Here is what he did:

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--;
    }
}

Wow, huh? It's not so easy to decide whether refactoring really helped make the code more readable. The feeling that, in fact, it has become more difficult to read. Previously, there was a relatively small function with useful comments, which was now turned into a class with four functions inside without comments. I would not say that the new class is bad and all refactoring was a bad idea, and the programmer who did the refactoring should be executed. Not at all. I'm not so bloodthirsty. There are several differences between these two code examples. Consider these differences:

1. If you are trying to achieve a deep understanding of what a top-level function does, the function has become more difficult to read than it was originally, because now you need to skip through all the functions and understand what is happening in each of them. On the contrary, the initial version can easily be quickly run through the eyes.
2. If you are trying to understand what makes a top-level function conceptually, then the refactored version is easier to read, since we immediately see what the function conceptually does within itself.
3. The third difference that I see is the cost of support. As for our specific example, I would say that the cost of supporting the refactored version is higher than the initial one (at least you need to refactor). In general, the answer to the question of which option is more expensive to support lies in the plane of requirements. These requirements dictate to us whether it is important in this situation to follow the SRP (principle of sole responsibility) or not. If this function can be written and forgotten once before the end of time, then there is no reason to waste time refactoring it. On the contrary, if functionality is expected to grow, then you have every reason to refactor a function into a separate class.

In addition, I want to touch on a situation where you accidentally (or intentionally) stumble upon a similar function in a legacy system. You immediately rush to extract a class with four functions inside? My advice is - do not do this without any reason, even if the coverage of tests on your code base tends to 100%. Why? Because there is no technical debt. I am talking about a serious technical debt that causes suffering.

Thus, there is nothing wrong with the extract till you drop technique. You just have to keep in mind some considerations, in my opinion.
To summarize, I want to say that you should never commit meaningless acts. You must first think, analyze, draw a conclusion, and only then act.