Method extraction vs underlying assumptions

Question

When I split big methods (or procedures, or functions — this question is not specific to OOP, but since I work in OOP languages 99% of the time, it's the terminology that I'm most comfortable with) into a lot of small ones, I often find myself displeased with the results. It becomes harder to reason about these small methods than when they were just blocks of code in the big one, because when I extract them, I lose a lot of underlying assumptions that come from the context of the caller.

Later, when I look at this code and see individual methods, I don't immediately know where are they called from, and think about them as ordinary private methods that can be called from anywhere in the file. For example, imagine an initialisation method (constructor or otherwise) split into a series of small ones: in the context of method itself, you clearly know that object's state is still invalid, but in an ordinary private method you probably go from assumption that object is already initialised and is in a valid state.

The only solution I've seen for this is the where clause in Haskell, which allows you to define small functions that are used only in the "parent" function. Basically, it looks like this:

len x y = sqrt $ (sq x) + (sq y)
    where sq a = a * a

But other languages I use don't have anything like this — the closest thing is defining a lambda in a local scope, which is probably even more confusing.

So, my question is — do you encounter this, and do you even see this is a problem? If you do, how do you typically solve it, particularly in "mainstream" OOP languages, like Java/C#/C++?

Edit about duplicates: As others noticed, there are already questions discussing splitting methods and small questions that are one-liners. I read them, and they don't discuss the issue of underlying assumptions that can be derived from caller's context (in example above, object being initialised). That's the point of my question, and that's why my question is different.

Update: If you followed this question and discussion underneath, you might enjoy this article by John Carmack on the matter, in particular:

Besides awareness of the actual code being executed, inlining functions also has the benefit of not making it possible to call the function from other places. That sounds ridiculous, but there is a point to it. As a codebase grows over years of use, there will be lots of opportunities to take a shortcut and just call a function that does only the work you think needs to be done. There might be a FullUpdate() function that calls PartialUpdateA(), and PartialUpdateB(), but in some particular case you may realize (or think) that you only need to do PartialUpdateB(), and you are being efficient by avoiding the other work. Lots and lots of bugs stem from this. Most bugs are a result of the execution state not being exactly what you think it is.

Answer 1

For example, imagine an initialisation method split into a series of small ones: in the context of method itself, you clearly know that object's state is still invalid, but in an ordinary private method you probably go from assumption that object is already initialised and is in a valid state. The only solution I've seen for this is...

Your concern is well-founded. There is another solution.

Take a step back. What fundamentally is the purpose of a method? Methods only do one of two things:

• Produce a value
• Cause an effect

Or, unfortunately, both. I try to avoid methods that do both, but plenty do. Let's say that the effect produced or the value produced is the "result" of the method.

You note that methods are called in a "context". What is that context?

• The values of the arguments
• The state of the program outside of the method

Essentially what you are pointing out is: the correctness of the result of the method depends on the context in which it is called.

We call the conditions required before a method body begins for the method to produce a correct result its preconditions, and we call the conditions which will be produced after the method body returns its postconditions.

So essentially what you are pointing out is: when I extract a code block into its own method, I am losing contextual information about the preconditions and postconditions.

The solution to this problem is make the preconditions and postconditions explicit in the program. In C#, for instance, you could use Debug.Assert or Code Contracts to express preconditions and postconditions.

For example: I used to work on a compiler which moved through several "stages" of compilation. First the code would be lexed, then parsed, then types would be resolved, then inheritance hierarchies would be checked for cycles, and so on. Every bit of the code was very sensitive to its context; it would be disastrous, for instance, to ask "is this type convertible to that type?" if the graph of base types was not yet known to be acyclic! So therefore every bit of code clearly documented its preconditions. We would assert in the method that checked for type convertibility that we had already passed the "base types acylic" check, and it then became clear to the reader where the method could be called and where it could not be called.

Of course there are lots of ways in which good method design mitigates the problem you've identified:

• make methods that are useful for their effects or their value but not both
• make methods that are as "pure" as possible; a "pure" method produces a value that depends only on its arguments, and produces no effect. These are the easiest methods to reason about because the "context" they need is very localized.
• minimize the amount of mutation that happens in program state; mutations are points where code gets harder to reason about

Answer 2

I often see this, and agree that it is a problem. Usually I resolve it by creating a method object: a new specialized class whose members are the local variables from the original, too-large method.

The new class tends to have a name like 'Exporter' or 'Tabulation', and it gets passed whatever information is necessary to do that particular task from the larger context. Then it is free to define even smaller helper code snippets that are in no danger of being used for anything but tabulating or exporting.

Answer 3

Many languages let you nest functions like Haskell. Java/C#/C++ are actually relative outliers in that regard. Unfortunately, they are so popular that people come to think, "It has to be a bad idea, otherwise my favorite 'mainstream' language would allow it."

Java/C#/C++ basically think a class should be the only grouping of methods you ever need. If you have so many methods that you can't determine their contexts, there are two general approaches to take: sort them by context, or split them by context.

Sorting by context is one recommendation made in Clean Code, where the author describes a pattern of "TO paragraphs." This is basically putting your helper functions immediately after the function that calls them, so you can read them like paragraphs in a newspaper article, getting more details the further you read. I think in his videos he even indents them.

The other approach is to split your classes. This can't be taken very far, because of the annoying need to instantiate objects before you can call any methods on them, and inherent problems with deciding which of several tiny classes should own each piece of data. However, if you've already identified several methods that really only fit in one context, they are probably a good candidate to consider putting into their own class. For example, complex initialization can be done in a creational pattern like builder.

Answer 4

I think the answer in most cases is context. As a developer writing code, you should assume your code is going to be changed in the future. A class might be integrated with another class, might replace it's internal algorithm, or might be split off to several class in order to create abstraction. Those are things beginner developers usually don't take into consideration, causing a need for messy workarounds or complete overhauls later.

Extracting methods is good, but to some degree. I always try to ask myself these questions when inspecting or before writing code:

• Is this code only used by this class/function? will it stay the same in the future?
• If I'll need to switch out some of the concrete implementation, can I do it easily?
• Can other developers on my team understand what's done in this function?
• Is the same code used somewhere else in this class? you should avoid duplication in almost all cases.

In any case, always think single responsibility. A class should have one responsibility, it's functions should serve one single constant service, and if they do a number of actions, those actions should have their own functions, so it's easy to differentiate or change them later.

Answer 5

It becomes harder to reason about these small methods than when they were just blocks of code in the big one, because when I extract them, I lose a lot of underlying assumptions that come from the context of the caller.

I didn't realize how big of a problem this was until I adopted an ECS which encouraged bigger, loopy system functions (with systems being the only ones having functions) and dependencies flowing towards raw data, not abstractions.

That, to my surprise, yielded a codebase so much easier to reason about and maintain compared to the codebases I worked in during the past where, during debugging, you had to trace through all kinds of teeny little functions, often through abstract functions calls through pure interfaces leading to who knows where until you trace into it, only to spawn some cascade of events which lead to places you never thought the code should ever lead.

Unlike John Carmack, my biggest problem with those codebases wasn't performance since I never had that ultra-tight latency demand of AAA game engines and most of our performance issues related more to throughput. Of course you can also start to make it more and more difficult to optimize hotspots when you're working in narrower and narrower confines of teenier and teenier functions and classes without that structure getting in the way (requiring you to fuse all these teeny pieces back to something bigger before you can even begin to effectively tackle it).

Yet the biggest issue for me was being unable to confidently reason about the system's overall correctness in spite of all tests passing. There was too much to take into my brain and comprehend because that type of system didn't let you reason about it without taking into account all these tiny details and endless interactions between tiny functions and objects that were going on everywhere. There were too many "what ifs?", too many things that needed to be called at the right time, too many questions about what would happen if they were called the wrong time (which start to become raised to the point of paranoia when you have one event triggering another event triggering another leading you to all kinds of unpredictable places), etc.

Now I like my big ass 80-line functions here and there, as long as they're still performing a singular and clear responsibility and don't have like 8 levels of nested blocks. They lead to a feeling that there are less things in the system to test and comprehend, even if the smaller, diced up versions of these bigger functions were only private implementation details not able to be called by anyone else... still, somehow, it tends to feel like there's less interactions going on throughout the system. I even like some very modest code duplication, as long as it's not complex logic (say just 2-3 lines of code), if it means less functions. I like Carmack's reasoning there about inlining making that functionality impossible to call elsewhere in the source file. There's something there to it when you have a shallower call stack and bigger, meatier functions and objects... a "flatter" system, not a "deeper" one.

Simplicity doesn't always reduce complexity at the big picture level if the option is between one meaty function vs. 12 uber-simple ones which call each other with a complex graph of dependencies. At the end of the day you often have to reason about what goes on beyond a function, have to reason about what these functions add up to ultimately do, and it can be harder to see that big picture if you have to deduce it from the smallest puzzle pieces.

Of course very general-purpose library type code that's well-tested can be exempt from this rule, since such general-purpose code often functions and stands well on its own. Also it tends to be teeny compared to the code a bit closer to the domain of your application (thousands of lines of code, not millions), and so widely applicable that it starts to become a part of the daily vocabulary. But with something more specific to your application where the system-wide invariants you have to maintain go far beyond a single function or class, I tend to find it helps to have meatier functions for whatever reason. I find it much easier working with bigger puzzle pieces in trying to figure out what's going on with the big picture.

Answer 6

I don't think it's a big issue, but I agree it's troublesome. Usually I just place the helper immediately after its beneficiary and add a "Helper" suffix. That plus the private access specifier should make its role clear. If there's some invariant that doesn't hold when the helper is called, I add a comment in the helper.

This solution does have the unfortunate drawback of not capturing the scope of the function it helps. Ideally your functions are small so hopefully this doesn't result in too many parameters. Normally you'd solve this by defining new structs or classes to bundle up the parameters, but the amount of boilerplate required for that can easily be longer than the helper itself, and then you're back where you started with no obvious way of associating the struct with the function.

You already mentioned the other solution - define the helper inside the main function. It may be a somewhat uncommon idiom in some languages, but I don't think it'd be confusing (unless your peers are confused by lambdas in general). This only works if you can define functions or function-like objects easily though. I wouldn't try this in Java 7, for example, since an anonymous class requires introducing 2 levels of nesting for even the smallest "function". This is as close to a let or where clause as you can get; you can refer to local variables before the definition and the helper can't be used outside of that scope.