Solving Embarrassingly Obvious Problems In Erlang

This is revised version of my 2012 essay on using small, simple functions to drive clear-headedness in programming. After seven years and 8 million new career programmers, it’s time to dust it off.

There are two main reasons I like Erlang:

• The Erlang VM is an operating system for your code
• The Erlang language encourages rigor in problem solving

I’ll be discussing the second point here.

Refactoring Using Functions

Here’s an expression I use in life:

if
  joy(Activity) >= MinThreshold ->
    do(Activity);
  true ->
    skip_it
end

This is an example of Erlang’s if expression. The true clause is effectively the else clause in other languages — it always matches and is therefore evaluated when the previous clauses are false.

If you find this syntactically hard to process, you’re not alone.

Aside from illustrating the point that you may want to avoid if expressions in Erlang, this examples serves a higher purpose: it’s a straw man we can refactor to make things obvious.

Here’s another version, which replaces the if expression with a chain of function calls:

maybe_do(
  if_standard_met(joy(Activity), MinThreshold),
  Activity)

This version is equivalent to the first, but formalizes the logic of the “joy comparison” using the function if_standard_met. That logic is defined in the first version as language semantics using an if expression. In this version it shows up as application logic.

Here’s the process I used for the refactor:

• I read the first version and asked, “what is actually going on here?”
• After some thought I concluded that “some standard is being applied to an activity ”
• Using functions, I wrote what I thought is actually going on
• I read the final version a few times and became all the more convinced of its truth

This technique — decomposing complex functions into smaller constituent functions — can be used to recast hard-to-solve problems into a series of embarrassingly obvious problems. Erlang in particular lends itself well to this but the method can be applied to any language.

Myths About Erlang

In the Peabody Award winning video The Ghetto, the protagonist is confronted with several terrible obstacles that stand between him and the mythical technology he seeks.

Some obstacles one might face in Erlang include:

• Single assignment (immutable) variables
• No loops
• No return statement
• Bizarre if expressions

I wonder how many developers have fled in horror, having encountered these on their journey to use Erlang? It’s a pity, because each of these obstacles is, in fact, a feature that would have helped them become better software developers!

The gist is this:

• The problems presented by each obstacle can be solved using functions
• If you don’t use functions to overcome these obstacles, your code will indeed look terrible, even hideous
• If you take the effort to write functions that clearly describe your problems, your code will look great
• In Erlang, great looking code is often great, while terrible looking code is always terrible

If you adopt this One True Religion, you’ll discover something amazing: you can generally spot quality code by looking at a single metric — lines of code per function.

If your functions have more than a few lines of code (a maximum of four to five lines per function is a good benchmark), you need to look more closely — chances are you have an opportunity to factor them into smaller, more tightly focused functions.

But why?

Functions describe the problem you’re solving. If your functions contain too many lines, chances are you’re solving multiple problems without articulating them. If you take the opportunity to articulate them, you’ll have to think rigorously.

Thinking rigorously about problems is the process of software development.

You know you’re done when there’s nothing left to think about — the problem you’re solving becomes boringly trivial and the code is obviously correct.

True Religion.

A Real Life Example — Factoring Terrible Code

Once upon a time, I ran across code that I had written a year earlier. I obviously didn’t have religion when I wrote it. It’s easy to recognize as terrible code because it’s impossible to understand at a glance. Try it:

handle_amqp(#message{name="db.create"}=Msg, State) ->
  log:info({db_create, service:to_proplist(Msg)}),
  Name = get_required_attr("name", Msg),
  verify_db_name(Name),
  User = get_required_attr("user", Msg),
  Pwd = get_required_attr("password", Msg),
  Opts =
    case get_attr("cluster", Msg) of
      undefined -> [];
      Cluster -> [{cluster, Cluster}]
    end,
  case mysql_controller:create_db(Name, User, Pwd, Opts) of
    {ok, HostInfo} ->
      Attrs = [{"slaves", ""}|host_info_attrs(HostInfo)],
      {reply, message_response(Msg, Attrs), State};
    {error, Err} ->
      log:error({db_create, Err, erlang:get_stacktrace()})
      {error, Err, State}
  end.

This code might work but I have no idea — I’d need to subject it to a barrage of tests, or run it in production for a while. Worse, I can’t possibly know if this code is correct — I don’t even know what it’s meant to do!

While I think this code is “clean” syntactically (it doesn’t make my eyes bleed the way Erlang code can) the problem is not with the syntax — it’s with the meaning. To understand the code, I need to do some work.

What a great opportunity on a Sunday afternoon to think rigorously!

Step 1 — What’s Going On Here?

I’m going to factor this using a top-down decomposition. The coding mechanics are very simple: describe what’s going on as clearly as possible. As we’ll see again and again, the work here consists of answering the question: What’s really going on here?

So what’s really going on with this terrible function?

Simple: I’m handling a particular message type — “db.create”.

So let’s do that:

handle_amqp(#message{name="db.create"}=Msg, State) ->
  handle_db_create_msg(Msg, State).

Done!

Seriously. I’m done here! I’ve solved my problem completely and unequivocally. There’s no debating. There’s no head scratching. I don’t even need to test this — I’m completely done!

Step 2 — Handling “DB Create”

Okay, I’m not done. I need to handle the “DB Create” message, whatever that means.

Moving the original code into a separate function, I have this:

handle_db_create_msg(Msg, State) ->
  log:info({db_create, service:to_proplist(Msg)}),
  Name = get_required_attr("name", Msg),
  verify_db_name(Name),
  User = get_required_attr("user", Msg),
  Pwd = get_required_attr("password", Msg),
  Opts =
    case get_attr("cluster", Msg) of
      undefined -> [];
      Cluster -> [{cluster, Cluster}]
    end,
  case mysql_controller:create_db(Name, User, Pwd, Opts) of
    {ok, HostInfo} ->
      Attrs = [{"slaves", ""}|host_info_attrs(HostInfo)],
      {reply, message_response(Msg, Attrs), State};
    {error, Err} ->
      log:error({db_create, Err, erlang:get_stacktrace()}),
      {error, Err, State}
  end.

What’s going on here?

Let me think a moment, out loud:

• First I log the pending operation
• Then I collect and validate input from the message
• Then I create the database
• Then I handle the result

So after all, this is actually pretty simple — but you wouldn’t know it without reading the code carefully! Let’s use functions to make it completely obvious.

After some tinkering and a few misfires I came up with this:

handle_db_create_msg(Msg, State) ->
  log_operation(db_create, Msg),
  Args = db_create_args(Msg),
  Result = db_create(Args),
  handle_db_create_result(Result, Msg, State).

Take a moment and compare it to my “thinking out loud” exercise above. It’s exactly what’s going on — represented by four distinct, easy to read, meaningful functions.

Funny, this little piece of code took work to get right! It’s one of those “short letters” that we actually took the time for.

But I’m done!

Step 3 — Logging

Not done. I need a log_operation function.

log_operation(Type, Msg) ->
  log:info({Type, service:to_proplist(Msg)}).

I could easily do without this function — but it’s two lines of code that makes “log an operation” drop-dead obvious wherever you see it. Otherwise you’d be reading “call log:info with some args, hang on, what are these args again, I remember this from before, crap” each time.

Step 4 — Converting “DB Create” Message to Args

Next we’ll implement db_create_args.

Here’s what I have, mostly copied from the original function:

db_create_args(Msg) ->
  Name = get_required_attr("name", Msg),
  verify_db_name(Name),
  User = get_required_attr("user", Msg),
  Pwd = get_required_attr("password", Msg),
  Opts =
    case get_attr("cluster", Msg) of
      undefined -> [];
      Cluster -> [{cluster, Cluster}]
    end,
  [Name, User, Pwd, Opts].

Looking at the last line, this function returns a list of arguments that can be used downstream to create the database.

But it’s too much code for one function! It’s too much to read and process in one go. Let’s use functions to make this obviously correct.

What’s going on here? Turns out, not much:

db_create_args(Msg) ->
  [db_name(Msg),
   db_user(Msg),
   db_password(Msg),
   db_create_opts(Msg)].

It takes about two seconds to scan the function and shrug at its boringness. No squinting or head scratching. Too obvious. Moving on.

Here are the arg-for-message functions:

db_name(Msg) ->
  verify_db_name(get_required_attr("name", Msg)).
db_user(Msg) ->
  get_required_attr("user", Msg).
db_password(Msg) ->
  get_required_attr("password", Msg).
db_create_opts(Msg) ->
  case get_attr("cluster", Msg) of
    undefined -> [];
    Cluster -> [{cluster, Cluster}]
  end.

Why all the ceremony? What’s really wrong with the earlier version — the one where all the logic is floating in a monolithic block of code?

It’s only wrong in the moral sense — this is a religion! Remember, we’re after embarrassingly obvious. Each arg-for-message function takes a passing glance and is so anticlimactic that it’s hardly worth reading. That’s not the case with the monolith, which requires language parsing and inference before can even start to reason about lofty concepts like correctness.

But hang on a minute, that case expression in db_create_options is bothering me. I bet there’s some logic hidden in there that I haven’t thought about.

What’s going on there? I read this: if a “cluster” is specified in the message, I want to provide that cluster as an option. It’s really cluster options.

So let’s make it that:

db_create_opts(Msg) ->
  db_create_cluster_opts(get_attr("cluster", Msg)).
db_create_cluster_opts(undefined) -> [];
db_create_cluster_opts(Cluster)   -> [{cluster, Cluster}].

This is arguably an extreme focus on minutiae. Honestly, in practice I might have skipped this one. But here’s the difference:

• The first version — the one using a case expression — asks that you first process the the Erlang language semantics (case expression), then infer the meaning (what the case expression is doing), then consider the application logic.
• The second version — the one using a carefully named function — asks only that you consider the application logic. Yes, you have to read the function and pause for a moment to consider what “db create cluster options” means — and that’s is precisely the point! Once that logic clicks, it’s so boring and obvious you’re compelled to move on to more interesting topics.

Step 5 — Create the Database

Next is db_create:

db_create([Name, User, Pwd, Opts]) ->
  mysql_controller:create_db(Name, User, Pwd, Opts).

This is simply a pass-through to the external function call.

While we could live without this indirection, it provides an important role in making things obvious. The inputs to this operation are created by db_create_args and the result is handled by handle_db_create_result. The function db_create is logic part of that vocabulary. Yes, it hides the implementation detail of using mysql_controller. But the main benefit shows up in the clarity we see in handle_db_create_msg. So we spend an extra line of code.

In our One True Religion, it’s no sin to trade key strokes, which are cheap, for clarity, which is precious.

Step 6 — Handle the Database Create Result

Most Erlang developers use case expressions to handle function results. There’s nothing wrong with this:

case read_something() of
  {ok, Result}   -> Result;
  {error, Error} -> error(Error)
end

But now curious, you ask, “what’s really going on here?”

Well, I’m translating the result of read_something into something else.

So I’ll say that:

translate_read_something(read_something())

And then do it:

translate_read_something({ok, Result})   -> Result;
translate_read_something({error, Error}) -> error(Error).

If you’re using case expressions to do something, you’re missing an opportunity to name that something. Use a function. You’ll often be shocked to discover hidden, important logic during the naming process.

Of course translate_read_something is a terrible name — you want to clarify what something is — and that’s precisely the point! When you use functions this way, you’re forced to grapple with names. When something isn’t clear in a name, it isn’t clear in your brain.

In this example, we’re handling the result of a function. In such cases, I tend to use a handle_xxx naming convention — so something like handle_read_result.

With that background, here’s what I have for the db_create result handler:

handle_db_create_result({ok, HostInfo}, Msg, State) ->
  Attrs = [{"slaves", ""}|host_info_attrs(HostInfo)],
  {reply, message_response(Msg, Attrs), State};
handle_db_create_result({error, Err}, _Msg, State) ->
  log:error({db_create, Err, erlang:get_stacktrace()}),
  {error, Err, State}.

This is a mechanical refactoring of the case expression in the original handle_amqp function. It’s not bad — just a few lines of code. But take a moment to read the code. Is it obvious? Or does it look more like a stereoscopic image where, if you relax your gaze, a three dimensional Jesus emerges from the screen?

What’s going on here?

There are two scenarios: an ok case where the create operation succeeds and an error case where the create operation fails.

So let’s say that:

handle_db_create_result({ok, HostInfo}, Msg, State) ->
  handle_db_created(HostInfo, Msg, State);
handle_db_create_result({error, Err}, _Msg, State) ->
  handle_db_create_error(Err, State).

First, our success case handler:

handle_db_created(HostInfo, Msg, State) ->
  Attrs = [{"slaves", ""}|host_info_attrs(HostInfo)],
  {reply, message_response(Msg, Attrs), State}.

Not much code, but the code is weird. What’s going on with the “slaves” attribute? I’m not sure actually. Remember, this is real code that I’m refactoring — I had written this a year prior and don’t recall why it’s there.

Ah, I remember — it’s there to maintain a service level contract! The original API included a list of “slaves” (an unfortunate anachronism still used in many database applications). The new API does not. Rather than just drop the information, I’m providing an empty value.

But I think that’s a flaw: it’s not obvious that I’m implementing a backward compatibility layer. Sure, I could add a comment but why not just make the code obvious?

First, let’s re-spell our handler:

handle_db_created(HostInfo, Msg, State) ->
  Attrs = db_created_response_attrs(HostInfo),
  {reply, message_response(Msg, Attrs), State}.

Without a close look you may have missed the change: we converted implicit application logic “construct DB created response attrs” into a function that names that logic explicitly.

Here’s that function:

db_created_response_attrs(HostInfo) ->
  db_created_legacy_attrs(host_info_attrs(HostInfo)).

Looky there! The legacy behavior is now an official part of the implementation, rather than drifting haphazardly in an unnamed expression. If I ever forget about that weird bit of logic, I need only glance over the code. And while I might pause to think about the sensibility of this logic — that’s precisely the point! Our goal is to shift the mental burden away from language parsing and inference toward direct reasoning about application logic.

Here’s the entirely uninteresting implementation of that legacy support:

db_created_legacy_attrs(Attrs) -> [{"slaves", ""}|Attrs].

And finally, our error case handler:

handle_db_create_error(Err, State) ->
  log:error({db_create, Err, erlang:get_stacktrace()}),
  {error, Err, State}.

While this code looks innocent enough, there’s actually a bug! I didn’t notice it earlier because when I see a wall-of-code I say to myself, “Wow, look at all that code. I wonder what it does? Does it work? I bet it sure does. If there was a bug, we would have caught it in the unit tests. I’m glad we have such great test coverage. Look at all that code. Neat!”

But when you’re looking at one or two lines of code in a carefully named function, bugs are easier to spot.

So what’s wrong?

Look aterlang:get_stracktrace()— at casual glance, it looks like it returns the current stack trace. Does that mean I’m going to log the stack trace of the current operation? Why do I care about that?

Reading the docs forerlang:get_stacktrace/0 I see:

Get the call stack back-trace (stacktrace) of the last exception in the calling process… If there has not been any exceptions in a process, the stacktrace is [].

This is certainly not what I want! I’m not handling an exception in this function, so the result is either [] or some random exception from a previous operation!

I suspect this code may have once lived inside an exception catch block, which would have made sense. But in some previous refactor it was lost amidst the surrounding code sprawl. Would a type system have caught this? Not likely. A linter? Not likely. This is actually a problem endemic to exceptions as a language feature! But with the code stripped of its noise, my brain had a better chance of catching it.

This is what I want:

log:error({db_create, Err})

And we’re done! Actually, truly done!

Refactor Summary

Here’s the original, terrible function:

handle_amqp(#message{name="db.create"}=Msg, State) ->
  log:info({db_create, service:to_proplist(Msg)}),
  Name = get_required_attr("name", Msg),
  verify_db_name(Name),
  User = get_required_attr("user", Msg),
  Pwd = get_required_attr("password", Msg),
  Opts =
    case get_attr("cluster", Msg) of
      undefined -> [];
      Cluster -> [{cluster, Cluster}]
    end,
  case mysql_controller:create_db(Name, User, Pwd, Opts) of
    {ok, HostInfo} ->
      Attrs = [{"slaves", ""}|host_info_attrs(HostInfo)],
      {reply, message_response(Msg, Attrs), State};
    {error, Err} ->
      log:error({db_create, Err, erlang:get_stacktrace()}),
      {error, Err, State}
end.

Here’s the refactored code:

handle_amqp(#message{name="db.create"}=Msg, State) ->
  handle_db_create_msg(Msg, State).
handle_db_create_msg(Msg, State) ->
  log_operation(db_create, Msg),
  Args = db_create_args(Msg),
  Result = db_create(Args),
  handle_db_create_result(Result, Msg, State).
log_operation(Type, Msg) ->
  log:info({Type, service:to_proplist(Msg)}).
db_create_args(Msg) ->
  [db_name(Msg),
   db_user(Msg),
   db_password(Msg),
   db_create_opts(Msg)].
db_name(Msg) ->
  verify_db_name(get_required_attr("name", Msg)).
db_user(Msg) ->
  get_required_attr("user", Msg).
db_password(Msg) ->
  get_required_attr("password", Msg).
db_create_opts(Msg) ->
  db_create_cluster_opts(get_attr("cluster", Msg)).
db_create_cluster_opts(undefined) -> [];
db_create_cluster_opts(Cluster)   -> [{cluster, Cluster}].
db_create(Name, User, Pwd, Opts) ->
  mysql_controller:create_db(Name, User, Pwd, Opts).
handle_db_create_result({ok, HostInfo}, Msg, State) ->
  handle_db_created(HostInfo, Msg, State);
handle_db_create_result({error, Err}, _Msg, State) ->
  handle_db_create_error(Err, State).
handle_db_created(HostInfo, Msg, State) ->
  Attrs = db_created_response_attrs(HostInfo),
  {reply, message_response(Msg, Attrs), State}.
db_created_response_attrs(HostInfo) ->
  db_created_legacy_attrs(host_info_attrs(HostInfo)).
db_created_legacy_attrs(Attrs) -> [{"slaves", ""}|Attrs].
handle_db_create_error(Err, State) ->
  log:error({db_create, Err}),
  {error, Err, State}.

Whooooa! This is better?? No way in Hades is this better!

Original “terrible” code:

• Functions: 1
• Lines of code: 19
• Average lines per function: 19

Refactored “obvious” code:

• Functions: 15
• Lines of code: 39
• Average lines per function: 2.6

Code bloat! 19 lines to 39 lines! “Fired, you twit!”

Whoa, simmer down there Sparky.

We actually reduced the lines of from 19 to just 5 — a 4x decrease!

This is our new function — what we’re actually doing:

handle_db_create_msg(Msg, State) ->
  log_operation(db_create, Msg),
  Args = db_create_args(Msg),
  Result = db_create(Args),
  handle_db_create_result(Result, Msg, State).

Our total line count did go up by 2x. This is bad if you argue that line count correlates positively to bug count, which of course it must if everything else is constant. But in this case, there’s more to the story — we thought rigorously about what we’re doing and made sure our code reflects that work.

Our added lines of code are paid off in spades by making said code embarrassingly obvious.

Cost and Benefit

There’s a clear cost to this approach: time spent making things obvious.

It’s true — a lot thought and effort went into this process — and that’s precisely the point!

Here are some of the benefits:

• Programmer intent is clear, helping you identify faulty reasoning sooner
• Programmers introduce fewer bugs initially
• Testing is more focused and easier to reason about
• Code is easier to debug because bugs occur in a coherent, well defined context

The best way to appreciate these benefits is to try this method and run it over a period of months. But start once and see how it feels.

There’s another benefit you’ll enjoy that is less obvious: You’ll get better and faster at writing obvious code in the first place.

Often the process of programming is to start with a fuzzy notion of what you’re doing, try something, fail, try something else, get frustrated, fail some more, get really frustrated, experiment some more, begin to better understand the problem, revise the problem, try some more, get some success, fail again, take a Valium, repeat this process N times, and then, bam!— it’s working. And then you apply this make it obvious refactoring method to codify your gloriously working code so that you and others feel entirely apathetic because it’s so boringly plainly clear.

But if you do the later — the make it embarrassingly obvious work — you’ll become a much better programmer. You’ll get faster at spotting incoherent code. In time, you’ll get faster at translating incoherent code into something simple and clear. In still more time, you’ll start to write that simple and clear code straight away — because your brain changes. It’s like learning a language. At some point, with enough practice, you stop thinking about speaking that language and just start using it.

And then the whole cost-benefit discussion is put to rest — it’s all benefit.

Summary

• One of Erlang’s best qualities is that it’s easy to spot terrible code — just look for the long functions
• You can write great code by whittling terrible code into smaller and smaller pieces, each obviously correct, simply by asking “what’s going on here” — “what’s really going on?” until you know exactly what’s going on, everywhere
• Obviously correct code will have fewer bugs
• Obviously correct code is easier to fix when it does have bugs (relaxing for a moment our definition of correct — this is a religion so we can do that)
• Testing is similarly a process of clarity-seeking
• If you practice this method, you’ll get better and faster at writing embarrassingly obvious code directly

A final thought.

People who complain about Erlang’s hideous syntax are writing long functions with lots of variable assignments, case statements, if statements — trying to force an imperative style of programming into a functional language.

You might be tempted to blame Erlang. But you know better now.

Let us set our judgments aside, for who among us is without terrible code? Our love for the obvious and the correct must emanate like a beacon of light. Our mission must remain ever clear: Relentlessly seek out the obvious, and then do the obvious, until only the obvious remains.

📝 Read this story later in Journal.

👩‍💻 Wake up every Sunday morning to the week’s most noteworthy stories in Tech waiting in your inbox. Read the Noteworthy in Tech newsletter.