Always Inline Single-Call Functions

Conventional coding rule wisdom says that functions can be too long and then must be broken into shorter ones. If the only reason to break an outer function is its length, then the inner functions will be declared in the same class as the outer function, are private and only called once:

struct A {
private:
    R inner1(A a_, B b_) {
        …
    }
    S inner2(C c_, D d_) {
        …
    }

public: // or whatever outer wants to be
    void outer() {
        …
        auto r = inner(a, b);
        auto s = inner(c, d);
        …
    }
};

What do we gain?

• inner1/2 get names, which are hopefully helpful to the reader of the code.
• The parameters can be renamed (in this case from a/b/c/d to a/b/c/d). This is a two-edged sword: the renaming may be inadvertent, and using the same names in outer and inner may actually improve readablity of the code.
• The code of outer is more concise. The reader can get an overview of what outer does without delving into details.

Besides these advantages, in our work at think-cell, we also discovered distinct disadvantages when it comes to reading and refactoring which I have not seen discussed elsewhere:

When refactoring, single-call functions are special because there they do not need to accommodate the needs of another caller. But finding out that inner is single-call takes effort, in particular because the code of inner is located away from outer.

Also, when deciding how to repackage code into multiple functions, successful code reuse is empirical evidence for having picked good packages. This evidence is missing for functions that are only called once. So there is a good chance that the chosen packaging turns out to be the wrong one for the future code reuse that we did not know about when deciding on the packaging.

To summarize, single-call functions make it easy to miss possible refactoring opportunities, but at the same time are likely to require refactoring later. And indeed, we have seen that this is not a good combination and resulted in overly complicated and/or duplicated code in our codebase.

So we made the decision that functions which are only called once from the same class they are declared in (otherwise data hiding will provide some evidence for a good packaging), have to be inlined.

But what about the good things that separation into functions brings? We can still have them:

• We can name functional blocks by putting them into a code block with a comment:

struct A {
    void outer() {
        …
        R r;
        { /*inner*/
            …
            r=…;
        }
        …
    }
};

• To hide the details of these inner code blocks when wanting to get an overview of the outer function, both editors we use, Visual Studio and XCode, support collapsing of blocks, with only the comment remaining visible.
• If we want to hide the local variables of the surrounding scope from the inner code block, or if we want to initialize a result variable, we can put the code block into an immediately executed lambda:

struct A {
    void outer() {
        …
        auto r = /*inner*/[this, &a_=a, &b_=b]() -> R {
            …
        }();
        …
    }
};

inner only captures this and an explicit list of variables. Other local variables are hidden. The variables can still be renamed, but doing so requires extra effort and thus is unlikely to be inadvertent.

To some, our inline single-call functions rule is heresy. But I believe it actually makes for better code.