#15 Inline All Code
Hi there 👋,
I find myself giving the same code review comments over and over again. One such comment boils down to: “consider inlining that”. As developers we’ve been taught to never repeat code, by the sage programmers of the before-times and the ninjas on hackernews. Don’t Repeat Yourself they said. And we’ve obeyed. So much so that I see code that goes… a bit overboard with it. Every function, method, constant & class that can be extract will be extracted! But too much of a good thing will corrode your codebase. Today I’ll argue in favor of inlining code, even if that means duplication. And I brought supporters.
What’s wrong with extracting code
Let’s first look at the costs of extracting a function
fun main() {
// some logic for 10 lines of code
// some more logic for 10 lines of code
}
Let’s extract the 2 pieces of logic
fun part1(): IntermediateResult {
// some logic for 10 lines of code
}
fun part2(input: IntermediateResult): FinalResult {
// some more logic for 10 lines of code
}
fun main() {
val intermediateResult = part1()
val finalResult = part2(intermediateResult)
}
With the trade-offs:
👍 Contract for
part1()
andpart2()
, clearly defined input & output.👎 we have to chose a name for
part1
andpart2
. Naming is hard.👎
part1
andpart2
can be called from anywhere in the program.👎
part1
andpart2
hide costs & side effects.
Why are 3 & 4 drawbacks? Imagine part1
has a side effect. Calling it multiple times will cause that side effect multiple times, a common source of subtle bugs.
For example:
fun part1(): IntermediateResult {
db.execute("insert into my_table ...")
// other logic
}
fun part2(input: IntermediateResult): FinalResult {
val rows = db.execute("select * from my_table ...")
// do something with rows
}
How will part2()
behave? We don’t know, because it depends on how often we invoked part1()
before invoking part2()
. The database interaction is just an example, the same happens to network requests, events tracked to an analytics system, etc. Extracting part1()
hides the side effects and thereby allows developers to freely invoke part1()
anywhere else in the program. That changes downstream behavior of part2()
, often resulting in subtle bugs.
Moreover, extracting part1()
hides the cost, e.g. for blocking IO or logic in render loops. Invoking part1()
freely incurs the performance cost, for example timeouts, dropped frames or outright failure of the system.
To be fair, this is all much less of a problem in pure functional code, where functions don’t have side effects and data is shared as immutable values instead of shared state. However, object-oriented programming languages encourage coding styles that suffer from the above problems. At least in the codebases I see at work and in open source projects.
So… what to do?
Inline it all.
…oh hello John Carmack (yes the John Carmack), this is a surprise… oh you want to say something? Go for it!
Inlining functions [..] 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. [..] 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. [..] The function that is least likely to cause a problem is one that doesn’t exist, which is the benefit of inlining it.
Remember how calling part1()
multiple times introduced bugs? Inlining avoids that problem by “not making it possible to call the function from other places”.
John also notes that inlining code makes code more explicit. I keep telling y’all to Make It Explicit for years. So I fully agree with John.
Awareness of all the code that is actually executing is important, and it is too easy to have very large blocks of code that you just always skip over while debugging, even though they have performance and stability implications.
[..]
I know there are some rules of thumb about not making functions larger than a page or two, but I specifically disagree with that now – if a lot of operations are supposed to happen in a sequential fashion, their code should follow sequentially.
[..]
Inlining code quickly runs into conflict with modularity and OOP protections, and good judgment must be applied. The whole point of modularity is to hide details, while I am advocating increased awareness of details.
Now these are 2 good reasons for inlining code: avoid bugs caused by hidden state changes & make the code more explicit. But there’s another good reason to inline code - or conversely a bad reason to extract code in the first place: the wrong abstraction.
Sandi Metz explains how we regularly end up with The Wrong Abstraction:
A programmer sees duplication and extracts it into an abstraction.
They chose a name for it (because they must).Later another programmer changes the abstraction to implement a new feature.
Repeat 2 several times → we end up with a wrong abstraction & a wrong name.
Sandi’s advice: inline it.
When dealing with the wrong abstraction, the fastest way forward is back. Do the following:
Re-introduce duplication by inlining the abstracted code back into every caller.
Within each caller, use the parameters being passed to determine the subset of the inlined code that this specific caller executes.
Delete the bits that aren't needed for this particular caller.
[..]
When the abstraction is wrong, the fastest way forward is back. This is not retreat, it’s advance in a better direction. Do it. You'll improve your own life, and the lives of all who follow.
I often see abstract classes and base classes, arguably they are wrong abstractions most of the time. It’s a pervasive anti-pattern in the Java development world. Developers routinely introduce a BaseException
, a BaseController
or a BaseActivity
in an effort to share code (= stay DRY). Sharing the code is a good idea, but abstract or base classes is the wrong approach most of the time. Why? Let me dust off my 20 year old copy of Effective Java… Here it is:
Favor composition over inheritance (see also wikipedia)
Prefer interfaces to abstract classes
To share behavior don’t reach for class hierarchies. Create a new function or object for that behavior. Compose that function/object with your existing function/object. 💥 You’ve shared code without any of that Base
nonsense.
If it’s too late, if you already have a wrong abstraction BaseThing
on your hand, do what Sandi said: inline it.
The fastest way forward is back.
This is not retreat, it’s advance in a better direction.
Hyperlinks
Coding Constructs I Now Avoid
Similar to inlining it all, Daniel B. Markham tells us about the programming language features he avoids, including object orientation
Sticking everything into types hurts, so I don't do that anymore. I respect and understand those who do, and I think given the paradigm of how they're coding they're doing the right thing, but heading down that road always seems to end in heartache, complexity, and pain, so I stopped doing that.
3 very OO things he avoids: Mutability, Inheritance and (big) Interfaces. What more can I say? The man’s arguments make prefect sense!
Hack the ladder: Scope, Skills & Responsibilities for modern software engineering
A great mental model for thinking about engineering levels, the levels map nicely to the levels of impact I described in issue #14 Reverse Engineer Your Manager - Part 2 and add % of the org:
L3-L4 → individual | 30-50 % of org
L5 → team / group | 25-35 % of org
L6 → section / department | 10 % of org
L7 → multiple departments | 1-5 % of org
L8 → whole company | < 0.5 % of org
I particularly like the mental model for levels of assistance
Guide: When guiding people one provides clear and direct pointers to solutions that others can follow.
Mentor: When mentoring people one provides suggestions and also space to others to explore their own paths.
Grow: When helping people grow one makes sure to set the general direction, but then gets the hell out of the way.
Many more goodies in this long read.
Engineering Productivity Can Be Measured - Just not How You’d Expect
OkayHQ wants to be the engineering manager’s dashboard with metrics that actually matter. Forget about velocity, individual contributions and delivery-on-estimated-time. They ask the right questions:
How much free, uninterrupted time does an engineer have to code?
How long is an engineer waiting on a response from another engineer's review?
How often do dev tools get in the way instead of helping accelerate work?
How often are engineers required to context switch, preventing deep work?
How often do engineers receive pages outside of business hours, interrupting their sleep or family life?
I like the metrics they suggest, from a maker perspective and a manager perspective. Things like uninterrupted “maker time”, time-to-code-review and CI time. I’d love to have a dashboard with these metrics for each of my teams.
Another OkayHQ team member was on SE Daily to go into more detail (50 min listen)
That’s it. If you enjoyed this ☝️ subscribe or share it with a friend.