Refactoring, Extraction, and Naming

The Importance of Good Names

Does naming really matter? Yes. It really, really does.

It's important that humans be able to read code -- maybe even more important than having computers read the code. Names help humans identify and make sense of concepts, otherwise we won't know what we're looking at. In addition, code is write-once, read-many-times. Names capture the knowledge we have now, so that future readers (who might be you!) can understand what's going on.

The Path to Good Names

Refactoring

Code rarely starts its life with great names. There aren't a lot of people who can name concepts perfectly while writing new functionality. Instead, we can defer naming until the "refactor" step of the red-green-refactor TDD cycle: write tests, make them pass, then once the code is definitely working, use the techniques below to pull out and name parts of your code. Since we wrote the code recently, we can immediately apply that knowledge of the code to writing the best possible names.

Extraction

It's good to think about naming in terms of extraction: if we pull out a chunk of code, then the chunk now needs a name in order to refer to it. Extraction is good and we should do it often: small methods and small classes are great and easier to understand.

The more names we can build into your application, the easier it is to build a mental model of what that code is and does.

Single Level of Abstraction

How do we know whether we're at a single level of abstraction?

Sandi Metz's talk on All the Little Things mentions a "Squint Test":

If we squint at the code (so it's a little blurry), are there a lot of different parts? Is there wildly varying indentation? Are there strings and regexes and methods all jumbled together? If so, the code may be doing too much and would be better as multiple pieces.

(The Atom editor has a plugin for this.)

Here's a picture of the Squint Test in action:

Avdi Grimm's Confident Code also talks about a single level of abstraction at a more semantic level: if the code that does thing A, thing B, and thing C are mixed together like A-B-A-C-B, it's harder to read than cohesive parts like A-A-B-B-C.

Extraction and Naming Examples

We have a Refactoring course right here on Upcase if you want more practice with this.

Extract Variable

Given this code:

def calculate(value)
  value * 0.07
end

Without context, we can't figure out what 0.07 means or why it's there. We could look at what methods call this one, the surrounding class, and go source digging -- but if we use good naming, we don't have to spend that time.

0.07 is a magic number. "Magic number" is a term for any primitive value (numbers, strings, regular expressions, etc) with unexplained meaning. We don't know why they're there, so we should extract and name them.

We can use the Extract Variable refactoring to put the 0.07 value into a well-named constant:

SALES_TAX = 0.07

def calculate(value)
  value * SALES_TAX
end

Now we know what this method does with much less effort.

We might also want to do this with other primitives like regular expressions:

content.gsub(/\s+$/, '')

We have to parse the regular expression, remember that \s means space, and take a few seconds to load its meaning into our brain.

Observe the power of good naming:

TRAILING_WHITESPACE_PATTERN = /\s+$/

content.gsub(TRAILING_WHITESPACE_PATTERN, '')

Extract Method

The next level of refactoring is Extract Method. This can do more than Extract Variable because we're extracting a full method, not a single expression.

This method is based on a few different conditions, one about a video and two about a user:

def show_sample?(video)
  video.has_sample? && \
    current_user.present? && !current_user.subscriber?
end

We can extract the user-specific parts to a method and in the process, define a domain concept: the sampler user. Here's what that looks like:

def show_sample?(video)
  video.has_sample? && sampler?
end

private

def sampler?
  current_user.present? && !current_user.subscriber?
end

Before we were checking the state of the world, but it wasn't clear why we needed to know these facts about the user. Now that we've named our domain concept, we know that it's because this user is a sampler.

Convert Comment to Method

Often we'll come across comments in code, like this:

# find the user based on the email in the params
user = User.find_by(email: params[:user_email])

Very often, that comment can be almost directly converted into a method:

user = find_user_by_email_param

# elsewhere in file

def find_user_by_email_param
  User.find_by(email: params[:user_email])
end

Comments go stale and comments lie. Method names can't go stale because they're part of the running application.

Real-life refactoring example

The de-factored (intentionally made worse) code sample from the Upcase code base has one very long method with a lot of domain logic (highlighted in white) mixed with Rails-specific code (in yellow).

The refactored version has a lot of small, easy-to-read methods that each operate at a single level of abstraction. You can see the refactored version in the Upcase repo.

For example, the new create method is high-level and can be parsed in much less time:

def create
  sign_in_user_from_auth_hash
  track_authed_to_access
  redirect_to_desired_path
  clear_return_to
  clear_auth_to_access_slug
end

We used the Extract Method refactoring (and even extracted methods from methods) to arrive at the code above.

Every line is high-level domain logic, and we can dive in to how it's implemented if we want to. The key is that we don't have to: we used good naming to explain what's going on without the code getting in the way. It reads like a story.

Note that the branch complexity (the if/else) in each method is still there, but it's harder to see. As a further refactoring we might try to reduce that complexity as well.

Maintainable code

Refactoring isn't just to DRY up code. It's also about maintainability: ensuring that working code will be maintainable in the future. A great way to make code maintainable is to make it easy to understand: try to make it read like you'd explain it to someone else.

What Makes for a Good Name?

We've talked about how to extract code and where to put it, but how do we decide what to name something?

Well, it depends: it depends on your business logic and your domain. Great names are often domain-specific, because part of their utility is to explain the domain.

A good name separates the idea of a thing from its implementation. Every name should introduce an abstraction and hide away unimportant (to that level) details.

The ReturnPathFinder is one example:

# Parses a URL for a `return_to` path.
class ReturnPathFinder
  def initialize(url)
    @url = url
  end

  def return_path
    query_string['return_to']
  end

  private

  attr_reader :url

  def query_string
    Rack::Utils.parse_nested_query(parsed_url.query)
  end

  def parsed_url
    URI.parse(url)
  end
end

It's a small, focused class at a single level of abstraction. It pulls out a specific query value from a URL. Its usage hides the implementation while being clear about what's happening at a high level:

ReturnPathFinder.new(auth_origin).return_path