---
title: Writing Less Error-Prone Code
teaser: |
  How to optimize for a living codebase, where a line of code will be repeatedly
  read, adapted and copied.
tags: code,good code
author: Oli Peate
published_on: 2018-06-08
---

How much time do you spend worrying about avoiding duplication in code you've
written? And how about when you review a pull request? Contrast that commitment
to this follow-up question:

**How much time do you spend thinking about making code less susceptible to
errors in the future?**

A line of code you write in a codebase under active development may be read
tens, hundreds or thousands of times. This line is likely to be adapted, moved
and copied multiple times. How should we optimize for this reality of a living
codebase?

## Daydream

Picture the scene: You've written well-tested code which completes a feature.
The resulting pull request is concise and clear; it's approved swiftly. The sun
in shining. It's time for lunch... perhaps today is a good day to try the new
burrito place?

Eight weeks later a colleague copies a test you wrote originally as the basis
for a new test. Regrettably a mistake is introduced — the freshly copied test is
missing an `after` hook to reset a global value after the test. However in
isolation the new test passes, as does the whole test suite during the review
process.

> Huh. A build failed on the CI server.

It's nine weeks after you wrote the code which was subsequently copied. You're
late for a meeting so you hit the re-build button and dash off. You promise
yourself to keep an eye out for that test failing again.

Fast-forward twelve months. The test suite only passes on CI about 60% of the
time. Team mates grumble about writing tests — what's the point anyway if they
keep flickering? You feel stressed.

I retract the previous paragraph. This isn't a horror story. A week after the
first re-build you noticed the test failing again. You use the failing seed
numbers to determine the combination and order of tests which cause the failure
and a fix is applied. Phew.

## Side Effects

Seemingly inconsequential decisions have potential side effects. Consider the
ramifications from removing the duplicate `after` hooks within each `context`:

```diff
RSpec.describe "holiday promotions" do
+ after { Timecop.return }

  context "on Independence day" do
    before { Timecop.freeze Date.parse("2017-07-04") }
-   after { Timecop.return }

    it "displays the fireworks banner" do
      # ...
    end
  end

  context "on Thanksgiving" do
    before { Timecop.freeze Date.parse("2017-11-23") }
-   after { Timecop.return }

    it "displays the turkey banner" do
      # ...
    end
  end

  context "on Black Friday" do
    before { Timecop.freeze Date.parse("2017-11-24") }
-   after { Timecop.return }

    it "displays the TV banner" do
      # ...
    end
  end
end
```

This change has an immediate trade-off on portability. One can no longer copy a
given `context` and re-use it in a new test file without causing time to be left
in an incorrect state.

While the original test file has few lines there's a solid chance the reader
will notice the `after` block and copy it across into the new test file. If the
original test file grows to hundreds of lines then the chances of missing the
`after` hook are greater.

For some it might feel instinctive to blame the individual copying the code for
not fully reading the original test. Or blame them for not knowing one should
always use `Timecop.return` after freezing time. But then again perhaps the pull
request reviewer should share some of the blame too.

Nope. None of these directions are valid or productive thoughts.

There's another approach. And it's not setting up a global `after` hook to
ensure time is always reset. That's bloat. Plus it fixes an *instance* of this
kind of bug. Another variant will occur.

## Making Seatbelts Mandatory

How can we make code from the example above less susceptible to errors when it's
re-used? A solution to this particular problem is using a block syntax:

```ruby
RSpec.describe "holiday promotions" do
  context "on Independence day" do
    it "displays the fireworks banner" do
      independence_day = Date.parse("2017-07-04")

      Timecop.freeze(independence_day) do
        visit(homepage_path)
      end

      expect(page).to have_content "50% off fireworks"
    end
  end

  context "on Thanksgiving" do
    it "displays the turkey banner" do
      thanksgiving = Date.parse("2017-11-23")

      Timecop.freeze(thanksgiving) do
        visit(homepage_path)
      end

      expect(page).to have_content "50% off turkeys"
    end
  end

  context "on Black Friday" do
    it "displays the TV banner" do
      black_friday = Date.parse("2017-11-24")

      Timecop.freeze(black_friday) do
        visit(homepage_path)
      end

      expect(page).to have_content "50% off TVs"
    end
  end
end
```

Using the block syntax ensures time is put back the way it was. Now there's no
risk of an `after` hook being omitted. The Timecop library used in these
examples lets you opt-in to a ['safe mode'][safe-mode] which only permits this
block syntax.

[safe-mode]: https://github.com/travisjeffery/timecop#timecopsafe_mode

When writing higher level tests, like those above, our minds need to be focused
on the end-user. Ideally we're free from the distracting details of how time
stubbing is implemented. Providing higher levels of abstraction and safety
aren't 'dumbing down' a codebase or dulling tools - it's friendlier and
practical. Every contributor gains when we find the right balance between
expressiveness and control for the task at hand.

A team working together on a codebase could establish a broader rule building
upon this solution: When code modifies global state in a test, prefer blocks to
ensure that global state is restored.

This 'do-then-tidyup' block syntax pattern is found throughout the Ruby standard
library too, from temporarily changing directories with [`Dir.chdir`][dir-chdir]
to closing a file after opening it with [`File.open`][file-open].

[dir-chdir]: https://ruby-doc.org/core-2.5.0/Dir.html#method-c-chdir
[file-open]: https://ruby-doc.org/core-2.5.0/File.html#method-c-open

## Readability

Another key aspect to avoiding bugs arising from code 'after it was born' (a.k.a
the majority of its life) is readability. If code can be read and understood
more readily then there's less opportunity for misunderstandings and mistakes.
Consider these two snippets of code:

```ruby
is_friday? ? display_burger_deal(:cheeseburger) : display_coffee_deal(:flat_white)
```

```ruby
if is_friday?
  display_burger_deal(:cheeseburger)
else
  display_coffee_deal(:flat_white)
end
```

Many developers are familiar with ternary statements. I'd wager familiarity with
the concept doesn't make it much easier for the reader's brain to digest what's
going on. Especially if they didn't write the code and have no context to lean
on. This example deliberately places a predicate method (`is_friday?`) next to
the ternary operator (`?`) for maximum brain-parsing awkwardness.

What's not to like about the second snippet? One might claim it's 'wasteful'
because it takes five lines to express what could be achieved in one. I think
this response stems from that initial thrill of learning about ternary
statements and feeling smart...

> WOW this is AWESOME. Hold on a moment while I use this EVERYWHERE.

I've felt that thrill too. Don't give into the smug conciseness of the the
one-line predicate. Be proud of emphasizing code branches — at the end of
the day comprehension matters.

## Communicating With Code

Every line of code is an opportunity to communicate with the reader and
demonstrate a style to replicate. Code is more than a set of instructions to a
machine. We need to meet additional goals - for humans - like making the code
easy to change and easy to grok, especially when our minds are preoccupied with
the task at hand.

Begin the journey towards these goals by embracing:

- Easy to understand code. In practice this means emphasizing readability over
  outright conciseness.
- Portability and 'copy & paste' reusability. Being able to cut / copy and paste
  a chunk of code without loss of correctness or meaning is underrated. This
  form of code reuse is seldom acknowledged but seems to happen all the time,
  within a codebase or between them.

Strive to write code which is infectiously good; it's readable, reusable, and
adaptable. At any point this code may need to be changed or called upon to act
as a kind of template — either extracted into another codebase or remixed within
the same codebase to address a whole new challenge.