How long should a function be? One level of abstraction long

Every developer has asked it at some point: how long should a function be? Ten lines? Twenty? Fits on one screen?

Line counts make poor answers, because they measure the wrong thing. The better answer is:

A function should be one level of abstraction long.

This is the Single Level of Abstraction Principle (SLAP). A function should stay at one level of detail. It should either describe what happens in broad strokes or spell out how a small piece of it works. It should not do both.

I ran into this while building QDVC, so I’ll use that code as the example.

The context: QDVC

DVC (Data Version Control) is an open-source tool for managing large datasets next to your code. It keeps small .dvc files in git, stores the real data in a cache and a remote repository, and lets you add, push and pull that data.

DVC was too slow for our needs, so I wrote QDVC (Quick DVC), a .NET command-line tool with a narrower scope:

• It only tracks data, with no experiments and no pipelines.
• It works on the existing DVC infrastructure: the same .dvc files, .dvc folder and config.
• Its commands are similar to DVC’s but not identical: status, pull, add, push and clean.

The interesting part for this post is the entry point, Main.

Before: a function that does everything

This is the first version of Main, trimmed a little:

static async Task<int> Main(string[] args)
{
    SystemContext.Initialize();
    Console.WriteLine($"Quick DVC v{VersionUtils.GetAssemblyInformationalVersion()}");
    var sw = Stopwatch.StartNew();
    IOContext.Initialize();

    CommandLineArguments Args = CommandLineArguments.ParseUsingSCL(args);
    if (Args.Command == null)
        return 0;

    if (!Args.Paths.Any())
    {
        Console.WriteLine("ERROR: No paths provided.");
        return 2;
    }

    var paths = Args.Paths.Select(Path.GetFullPath);
    var currentDirectory = FileSystem.Directory.GetCurrentDirectory();
    var dvcFolder = DvcCache.FindDvcRootForRepositorySubPath(currentDirectory) ??
                    DvcCache.FindDvcRootForRepositorySubPath(paths.First());
    if (dvcFolder == null)
    {
        Console.WriteLine("ERROR: No DVC repository found.");
        return 3;
    }

    var dvcConfig = DvcConfig.ReadConfigFromFolder(dvcFolder);
    var credentials = Credentials.DetectFrom(Args, dvcConfig);
    if (credentials == null)
    {
        Console.WriteLine("Failed to detect credentials.");
        return 4;
    }
    else
        Console.WriteLine($"Credentials loaded from {credentials.Source}");

    var files = paths.SelectMany(FilesEnumerator.EnumerateFilesFromPath);
    if (files.Any() == false)
    {
        Console.WriteLine("No files found.");
        return 5;
    }

    var cacheDir = dvcConfig.GetCacheDirAbsolutePath();
    var dvcCache = DvcCache.CreateFromFolder(cacheDir) ??
                   DvcCache.CreateFromRepositorySubFolder(files.First());
    Console.WriteLine($"DVC cache folder: {dvcCache?.DvcCacheFolder}");

    using var httpClient = new HttpClient();
    httpClient.Timeout = TimeSpan.FromMinutes(10);
    if (credentials != null)
        httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic",
            Convert.ToBase64String(Encoding.ASCII.GetBytes($"{credentials.Username}:{credentials.Password}")));

    switch (Args.Command)
    {
        case "pull":
            await new PullCommand(dvcCache, httpClient).ExecuteAsync(files);
            break;
        case "push":
            await new PushCommand(dvcCache, httpClient).ExecuteAsync(files);
            break;
        case "add":
            await new AddCommand(dvcCache).ExecuteAsync(files);
            break;
        // ... status, status--repo, clean ...
        default:
            Console.WriteLine($"Invalid command '{Args.Command}'");
            break;
    }

    Console.WriteLine($"Finished in {sw.Elapsed}");
    return 0;
}

The function works. It’s also about 70 lines of this: parse the arguments, check them, find the repository, read the config, look for credentials, check again, and so on, with an if and a return after almost every step. 70 lines, with multiple levels of ifs, which almost everyone would call it a “too long” function.

Reading it, you have to keep two things in mind at once:

1. The big picture: the program parses arguments, finds the repo, loads the config, gets credentials, finds the files, builds an HTTP client and runs a command.
2. The details: how to find the DVC folder (try the current directory, fall back to the first path), which exit code goes with which failure, how to build a Basic auth header out of a username and password.

But the function doesn’t feel “too long” because its 70 lines of code. The real cause is the jumping up and down to different level of abstractions, between the big picture (the higher level of abstraction) and the creation of a HttpClient (a lower level of abstraction).

These two levels are interleaved. The Convert.ToBase64String(Encoding.ASCII.GetBytes(...)) line that builds the auth header sits right above the switch (Args.Command) that decides what the program does. To understand the flow you have to read past all the mechanics, and to change one mechanic you have to scan the whole flow.

Two levels of abstraction

It helps to name the levels explicitly:

The original Main mixes both. SLAP says each function should stay on one of them.

After: a function you can read like a table of contents

Here is the refactored Main:

static async Task<int> Main(string[] args)
{
    SystemContext.Initialize();
    SystemContext.Console.StdOutWriteLine($"Quick DVC v{VersionUtils.GetAssemblyInformationalVersion()}");
    var sw = Stopwatch.StartNew();
    IOContext.Initialize();
    try
    {
        var arguments = ParseCommandLineArguments(args);
        var dvcFolder = FindDvcFolder(FileSystem.Directory.GetCurrentDirectory(), arguments);
        var dvcConfig = DvcConfig.ReadConfigFromFolder(dvcFolder);
        var credentials = FindCredentials(arguments, dvcConfig);
        var files = EnumerateFiles(arguments.Paths);
        var dvcCache = GetDvcCache(dvcConfig, files.First());
        using var httpClient = CreateHttpClient(dvcConfig, credentials);
        var command = CreateCommand(arguments, dvcCache, httpClient);
        await command.ExecuteAsync(files);

        SystemContext.Console.StdOutWriteLine($"Finished in {sw.Elapsed}");
        return 0;
    }
    catch (ExitException e)
    {
        if (e.ExitCode != 0)
            SystemContext.Console.StdErrWriteLine(e.Message);
        return e.ExitCode;
    }
    catch (Exception e)
    {
        SystemContext.Console.StdErrWriteLine($"ERROR: Unexpected error. {e.Message}");
        return 99;
    }
}

Main is now much shorter. More importantly, its core contains only things at the higher level of abstraction. Parse the arguments, find the DVC folder, read the config, find credentials, enumerate the files, get the cache, create the HTTP client, create the command, execute it. Someone seeing the program for the first time can understand its whole flow in about ten seconds.

The lower level didn’t go away. It moved into individual functions, and each of those also operates at a single level of abstraction: the lower one.

private static string? FindDvcFolder(string currentDirectory, CommandLineArguments arguments)
{
    var paths = arguments.Paths.Select(Path.GetFullPath);
    var dvcFolder = DvcCache.FindDvcRootForRepositorySubPath(currentDirectory) ??
                    DvcCache.FindDvcRootForRepositorySubPath(paths.First());
    if (dvcFolder == null)
        throw new ExitException(2, "ERROR: No DVC repository found.");
    return dvcFolder;
}

private static Credentials? FindCredentials(CommandLineArguments arguments, DvcConfig dvcConfig)
{
    var credentials = Credentials.DetectFrom(arguments, dvcConfig);
    if (credentials == null)
        throw new ExitException(3, "ERROR: Failed to find credentials.");
    SystemContext.Console.StdOutWriteLine($"Credentials loaded from {credentials.Source}".Color(LessImportant));
    return credentials;
}

Each of these does one small thing, including the error handling for that thing. The names are verbs describing intent, so the call site in Main reads almost like prose.

Notice how each extracted function acts as a bridge between two adjacent levels of abstraction. Its name lives at the higher level (FindCredentials is a step in the overall flow), while its body deals with the level immediately below (looking up the credentials, checking for null, throwing an error). The body should not reach further down than that. If it did, say by hand-rolling the base64 encoding, that detail would belong in a function of its own. This is what keeps each method at a single level of abstraction.

What did we actually gain?

1. Readability. You can read Main and learn what the program does without wading through how. When you need the how, you jump to the one function that owns it.

2. Error handling stops cluttering the flow. In the old version every step was followed by if (...) { Console.WriteLine(...); return N; }. In the new one the happy path is a straight line, and failures are throw new ExitException(code, message) inside the low-level function that detects them. A single catch in Main turns exceptions into exit codes. The mapping from failure to exit code still exists, but it now sits next to the code that knows about the failure.

3. A natural place for each change. If the Basic-auth header changes, you edit CreateHttpClient. If you add a command, you edit CreateCommand. Neither change touches the flow in Main.

4. Better names, which act as documentation. A function that does one thing at one level can have an honest name. A 70-line function can only be called something like Main.

5. Things become easier to see. Once the pieces were separated, the dependencies between them were visible. CreateCommand is a good example: it shows exactly which dependencies each command needs.

private static Command CreateCommand(CommandLineArguments arguments, DvcCache? dvcCache, HttpClient httpClient)
{
    Command command = arguments.Command switch
    {
        "pull" => new PullCommand(dvcCache, httpClient),
        "push" => new PushCommand(dvcCache, httpClient),
        "add" => new AddCommand(dvcCache),
        "status" => new StatusCommand(dvcCache),
        "status--repo" => new StatusRepoCommand(dvcCache, httpClient),
        "clean" => new CleanCommand() { Force = arguments.Force },
        _ => throw new ExitException(6, $"Invalid command '{arguments.Command}'")
    };

    if (arguments.OutputAbsolutePaths)
        command.FileNameFormatter = new AbsolutePathFileNameFormatter();

    return command;
}

At a glance: pull, push and status--repo talk to the remote, so they need the HTTP client. add and status only need the local cache. clean needs neither, only a Force flag. That was hard to spot when everything was one big function.

How to apply SLAP in practice

A few habits that helped me:

• Notice when you need to switch gears. If, while reading a function, you go from “what are the steps?” to “how do I encode this string?”, you’ve crossed a level boundary. That is a good place to extract a function.
• Name the extracted function after its intent, not its implementation. FindCredentials, not CheckEnvAndConfigAndReturnNullIfMissing.
• Keep error handling at the level of the thing that fails. A function that finds the DVC folder is the right place to say “no DVC repository found”. The top-level function is the right place to decide what to do with any failure.
• Don’t split mechanically. Extracting every three lines into a function is not SLAP, and you end up with a different kind of mess. The goal is that each function reads at one level, not that each function is tiny.
• Use the table-of-contents test. If the body of a function reads like an outline of the steps, it’s probably at a single level. If it reads like an outline with some paragraphs of detail in the middle, it isn’t.

Summary

Q: How long should a function be? A: One level of abstraction long.

Two questions are worth asking about any function:

1. Is everything in here at the same level of detail?
2. If not, what should be pulled down into its own function?

QDVC’s Main went from a 70-line mixture of flow and mechanics to nine lines of flow, backed by small functions that each deal with one mechanic. The program does exactly the same thing, but it is much easier to read, change and debug.