clean-code-ruby

clean-code-ruby

Clean Code concepts adapted for Ruby.

Inspired by clean-code-javascript.

Note: The examples are largely ported over from JavaScript so they may not be idiomatic. Feel free to point out any non-idiomatic Ruby code by submitting an issue and I'll correct it right away. Also, pull requests are always welcome!

Table of Contents

1. Introduction
2. Variables
3. Methods
4. Objects and Data Structures
5. Classes
6. SOLID
7. Testing
8. Error Handling
9. Formatting
10. Comments
11. Translations

Introduction

Software engineering principles, from Robert C. Martin's book Clean Code, adapted for Ruby. This is not a style guide. It's a guide to producing readable, reusable, and refactorable software in Ruby.

Not every principle herein has to be strictly followed, and even fewer will be universally agreed upon. These are guidelines and nothing more, but they are ones codified over many years of collective experience by the authors of Clean Code.

Our craft of software engineering is just a bit over 50 years old, and we are still learning a lot. When software architecture is as old as architecture itself, maybe then we will have harder rules to follow. For now, let these guidelines serve as a touchstone by which to assess the quality of the Ruby code that you and your team produce.

One more thing: knowing these won't immediately make you a better software developer, and working with them for many years doesn't mean you won't make mistakes. Every piece of code starts as a first draft, like wet clay getting shaped into its final form. Finally, we chisel away the imperfections when we review it with our peers. Don't beat yourself up for first drafts that need improvement. Beat up the code instead!

Variables

Use meaningful and pronounceable variable names

Bad:

yyyymmdstr = Time.now.strftime('%Y/%m/%d')

Good:

current_date = Time.now.strftime('%Y/%m/%d')

⬆ back to top

Use the same vocabulary for the same type of variable

Pick one word for the concept and stick to it.

Bad:

user_info
user_data
user_record

starts_at
start_at
start_time

Good:

user

starts_at

⬆ back to top

Use searchable names and use constants

We will read more code than we will ever write. It's important that the code we do write is readable and searchable. By not naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable.

Also, instead of hardcoding values and using "magic numbers", create constants.

Bad:

# What the heck is 86400 for?
status = Timeout::timeout(86_400) do
  # ...
end

Good:

# Declare them as capitalized globals.
SECONDS_IN_A_DAY = 86_400

status = Timeout::timeout(SECONDS_IN_A_DAY) do
  # ...
end

⬆ back to top

Use explanatory variables

Bad:

address = 'One Infinite Loop, Cupertino 95014'
city_zip_code_regex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/
save_city_zip_code(city_zip_code_regex.match(address)[1], city_zip_code_regex.match(address)[2])

Good:

address = 'One Infinite Loop, Cupertino 95014'
city_zip_code_regex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/
_, city, zip_code = city_zip_code_regex.match(address).to_a
save_city_zip_code(city, zip_code)

⬆ back to top

Avoid Mental Mapping

Explicit is better than implicit.

Bad:

locations = ['Austin', 'New York', 'San Francisco']
locations.each do |l|
  do_stuff
  do_some_other_stuff
  # ...
  # ...
  # ...
  # Wait, what is `l` for again?
  dispatch(l)
end

Good:

locations = ['Austin', 'New York', 'San Francisco']
locations.each do |location|
  do_stuff
  do_some_other_stuff
  # ...
  # ...
  # ...
  dispatch(location)
end

⬆ back to top

Don't add unneeded context

If your class/object name tells you something, don't repeat that in your variable name.

Bad:

car = {
  car_make: 'Honda',
  car_model: 'Accord',
  car_color: 'Blue'
}

def paint_car(car)
  car[:car_color] = 'Red'
end

Good:

car = {
  make: 'Honda',
  model: 'Accord',
  color: 'Blue'
}

def paint_car(car)
  car[:color] = 'Red'
end

⬆ back to top

Use default arguments instead of short circuiting or conditionals

Default arguments are often cleaner than short circuiting. Be aware that if you use them, your method will only provide default values for undefined arguments. Other "falsy" values such as false and nil will not be replaced by a default value.

Bad:

def create_micro_brewery(name)
  brewery_name = name || 'Hipster Brew Co.'
  # ...
end

Good:

def create_micro_brewery(brewery_name = 'Hipster Brew Co.')
  # ...
end

⬆ back to top

Methods

Method arguments (2 or fewer ideally)

Limiting the amount of method parameters is incredibly important because it makes testing your method easier. Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument.

One or two arguments is the ideal case, and three should be avoided if possible. Anything more than that should be consolidated. Usually, if you have more than two arguments then your method is trying to do too much. In cases where it's not, most of the time a higher-level object will suffice as an argument. Or you can pass data to the method by instance variables.

Since Ruby allows you to make objects on the fly, without a lot of class boilerplate, you can use an object if you are finding yourself needing a lot of arguments. The prevailing pattern in Ruby is to use a hash of arguments.

To make it obvious what properties the method expects, you can use the keyword arguments syntax (introduced in Ruby 2.1). This has a few advantages:

1. When someone looks at the method signature, it's immediately clear what properties are being used.
2. If a required keyword argument is missing, Ruby will raise a useful ArgumentError that tells us which required argument we must include.

Bad:

def create_menu(title, body)
  # ...
end

create_menu('Foo', 'Bar')

Good:

def create_menu(title:, body:)
  # ...
end

create_menu(title: 'Foo', body: 'Bar')

⬆ back to top

Methods should do one thing

This is by far the most important rule in software engineering. When methods do more than one thing, they are harder to compose, test, and reason about. When you can isolate a method to just one action, they can be refactored easily and your code will read much cleaner. If you take nothing else away from this guide other than this, you'll be ahead of many developers.

Bad:

def email_clients(clients)
  clients.each do |client|
    client_record = database.lookup(client)
    email(client) if client_record.active?
  end
end

email_clients(clients)

Good:

def email_clients(clients)
  clients.each { |client| email(client) }
end

def active(objects)
  objects.select { |object| active?(object) }
end

def active?(object)
  record = database.lookup(object)
  record.active?
end

email_clients(active(clients))

⬆ back to top

Method names should say what they do

Poorly named methods add to the code reviewer's cognitive load at best, and mislead the code reviewer at worst. Strive to capture the precise intent when naming methods.

Bad:

def add_to_date(date, month)
  # ...
end

date = DateTime.now

# It's hard to tell from the method name what is added
add_to_date(date, 1)

Good:

def add_month_to_date(date, month)
  # ...
end

date = DateTime.now
add_month_to_date(date, 1)

⬆ back to top

Methods should only be one level of abstraction

When you have more than one level of abstraction your method is usually doing too much. Splitting up methods leads to reusability and easier testing. Furthermore, methods should descend by the level of abstraction: one very abstract method should call methods that are less abstract and so on.

Bad:

def interpret(code)
  regexes = [
    # ...
  ]
  statements = code.split(' ')

  tokens = regexes.each_with_object([]) do |regex, memo|
    statements.each do |statement|
      # memo.push(...)
    end
  end

  ast = tokens.map do |token|
    # ...
  end

  ast.map do |node|
    # ...
  end
end

Good:

def interpret(code)
  tokens = tokenize(code)
  ast = lex(tokens)
  parse(ast)
end

def tokenize(code)
  regexes = [
    # ...
  ]
  statements = code.split(' ')

  regexes.each_with_object([]) do |regex, tokens|
    statements.each do |statement|
      # tokens.push(...)
    end
  end
end

def lex(tokens)
  tokens.map do |token|
    # ...
  end
end

def parse(ast)
  ast.map do |node|
    # ...
  end
end

⬆ back to top

Remove duplicate code

Do your absolute best to avoid duplicate code. Duplicate code is bad because it means that there's more than one place to alter something if you need to change some logic.

Imagine if you run a restaurant and you keep track of your inventory: all your tomatoes, onions, garlic, spices, etc. If you have multiple lists that you keep this on, then all have to be updated when you serve a dish with tomatoes in them. If you only have one list, there's only one place to update!

Oftentimes you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you to have two or more separate methods that do much of the same things. Removing duplicate code means creating an abstraction that can handle this set of different things with just one method/module/class.

Getting the abstraction right is critical, that's why you should follow the SOLID principles laid out in the Classes section. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make a good abstraction, do it! Don't repeat yourself, otherwise you'll find yourself updating multiple places anytime you want to change one thing.

Bad:

def show_developer_list(developers)
  developers.each do |developer|
    data = {
      expected_salary: developer.expected_salary,
      experience: developer.experience,
      github_link: developer.github_link
    }

    render(data)
  end
end

def show_manager_list(managers)
  managers.each do |manager|
    data = {
      expected_salary: manager.expected_salary,
      experience: manager.experience,
      portfolio: manager.mba_projects
    }

    render(data)
  end
end

Good:

def show_employee_list(employees)
  employees.each do |employee|
    data = build_data(employee)
    render(data)
  end
end

def build_data(employee)
  general_data = {
    expected_salary: employee.expected_salary,
    experience: employee.experience
  }

  general_data.merge(position_specific_data(employee))
end

def position_specific_data(employee)
  case employee.type
  when 'manager'
    { portfolio: employee.mba_projects }
  when 'developer'
    { github_link: employee.github_link }
  end
end

⬆ back to top

Don't use flags as method parameters

Flags tell your user that this method does more than one thing. Methods should do one thing. Split out your methods if they are following different code paths based on a boolean.

Bad:

def create_fil