Clean Code

Before We Start

This material is created for Go Academy program.

This slide is not meant to be a complete summary of the Clean Code book by Robert C. Martin.

This slide will only cover parts of the Clean Code book that are relevant to the Go Academy curriculum.

Meaningful Names

Why Names Matter

Names are everywhere in our codes. We name our variables, functions, methods, packages, files, etc. Because we names things constantly in programming, we better do it well.

Show Intent (1)

Good names reveal intent.

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

Non-revealing name:

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

We should choose a name that specifies what is being measured and the unit of that measurement:

Show Intent (2)

Names that reveal intent make it easier to understand and change code.

def get_it(a, b)
  if @cells[a]
    @cells[a][b]
  end
end

def cell_at(x, y)
  if @cells[x]
    @cells[x][y]
  end
end

Compare these two codes:

Pronounceable and Searchable

For the same type of variable, use consistent vocabulary.

class DtaRcrd
  def initialize(n, e)
    @name = n
    @email = e
  end
end

Bad:

class User
  def initialize(name, email)
    @name = name
    @email = email
  end
end

Good:

Class Names and Method Names

Classes and objects should have noun or noun phrase name.

class Customer
end

class WikiPage
end
def initialize
end

def process_payment
end

Methods should have verb or verb phrase name.

Pick One Word per Concept

When we have decided what name we use for a concept, use it consistently.

For instance, if we decide that to retrieve a record from database we use "get", use it consistently. Don't mix it with other verbs such as "fetch" or "retrieve".

Functions and Methods

Small

Good functions and methods are small.

In Go-Jek, we have a coding guideline that states that we should not write a function or a method that has more than 15 lines.

Do One Thing

Good functions and methods do one thing only but they do it well.

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

Bad:

def email_active_clients(clients)
  clients
    .select(&method(:active_client?))
    .each(&method(:email))
end

def active_client?(client)
  client_record = database.lookup(client)
  client_record.active?
end

Good:

Use Descriptive Names

Function and method names should be descriptive. A long but descriptive name is better than a short but enigmatic name.

def add_date(date, month)
end

Bad:

def add_month_to_date(date, month)
end

Good:

Prefer Fewer Arguments

Whenever possible, prefer functions and methods with fewer arguments.

def calculate_distance(source_point, destination_point)
end

Prefer:

def calculate_distance(x1, y1, x2, y2)
end

Than this:

Have No Side Effects

A function or method should not do anything other than what its name describe. State changing actions such as as writing files, storing to database, and changing a variable's value should never be done as a side effect. If we want our function or method to change a state, design it appropriately.

Don't Repeat Yourself (1)

Avoid duplicate code. Having duplicate code mean you have to change in multiple places when the logic of the code changes.

Rails' partials view is a good example of DRY principle in action.

Don't Repeat Yourself (2)

# new.html.erb
<h1>New Tag</h1>

<%= form_for(tag) do |f| %>
  <div class="field">
    <%= f.label :name %>
    <%= f.text_field :name %>
  </div>

  <div class="actions">
    <%= f.submit %>
  </div>
<% end %>

<%= link_to 'Back', tags_path %>

Before:

# edit.html.erb
<h1>Editing Tag</h1>

<%= form_for(tag) do |f| %>
  <div class="field">
    <%= f.label :name %>
    <%= f.text_field :name %>
  </div>

  <div class="actions">
    <%= f.submit %>
  </div>
<% end %>

<%= link_to 'Show', @tag %> |
<%= link_to 'Back', tags_path %>

Don't Repeat Yourself (3)

# new.html.erb
<h1>New Tag</h1>

<%= render 'form', tag: @tag %>

<%= link_to 'Back', tags_path %>

After:

# _form.html.erb
<%= form_for(tag) do |f| %>
  <div class="field">
    <%= f.label :name %>
    <%= f.text_field :name %>
  </div>

  <div class="actions">
    <%= f.submit %>
  </div>
<% end %>

# new.html.erb
<h1>New Tag</h1>
# edit.html.erb
<h1>Editing Tag</h1>

<%= render 'form', tag: @tag %>

<%= link_to 'Show', @tag %> |
<%= link_to 'Back', tags_path %>

Objects and Data Structure

Data Abstraction

When designing classes (that will be instantiated as objects), we want to keep our variables private and provide abstraction for the outside world to interact with our objects.

public class Point {
  public double x;
  public double y;
}

Bad:

public interface Point {
  public getX();
  public getY();
  void setCartesian(double x, double y);
}

Good:

Data/Object Anti-Simmetry

Objects hide their data behind abstraction and expose functions that operate on data.

Data structures expose their data and has no meaningful functions.

From this point of view, objects and data structures are an exact opposite.

The Law of Demeter (1)

A method f of a class C should only call the methods of these:

- C

- Objects created by f

- Objects passed as arguments to f

- Objects held in instance variables of C

The Law of Demeter (2)

product.provider.name  
provider.address.city
company.building.city

Bad:

class Product < ActiveRecord::Base
  belongs_to :provider

  def provider_name
    provider.name
  end
end

class Provider < ActiveRecord::Base
  has_many :products
end

product.provider_name

Good:

The Law of Demeter (3)

class Product < ActiveRecord::Base
  belongs_to :provider

  delegate :name, to: :provider, prefix: true
end

class Provider < ActiveRecord::Base
  has_many :products
end

product.provider_name

Even better, delegate:

Data Transfer Object

The quintessential form of a data structure is a class with public variables and no functions. This is sometimes called a data transfer object, or DTO.

Active Records are special forms of DTOs. They are data structures with public (or bean-accessed) variables; but they typically have navigational methods like save and find. Typically these Active Records are direct translations from database tables, or other data sources.

We will get to know more on Active Records when we learn Ruby on Rails later on.

Unit Tests

The Three Laws of TDD

When we are writing codes in TDD style, there are three important rules:

1. You may not write production code until you have written a failing unit test.

2. You may not write more of a unit test than is sufficient to fail, and not compiling is failing.

3. You may not write more production code than is sufficient to pass the currently failing test.

 

We will learn more about TDD in a separate session.

One Assert per Test (1)

Good tests only assert one thing per test.

require 'rspec'

describe 'Calculator' do
  let(:calculator) { Calculator.new }

  it 'performs addition, subtraction, multiplication and division' do
    expect(calculator.calculate('1 + 2')).to eq(3)
    expect(calculator.calculate('4 - 2')).to eq(2)
    expect(calculator.calculate('2 * 3')).to eq(6)
    expect(calculator.calculate('6 / 2')).to eq(3)
  end
end

Bad:

One Assert per Test (2)

require 'rspec'

describe 'Calculator' do
  let(:calculator) { Calculator.new }

  it 'performs addition' do
    expect(calculator.calculate('1 + 2')).to eq(3)
  end

  it 'performs subtraction' do
    expect(calculator.calculate('4 - 2')).to eq(2)
  end

  it 'performs multiplication' do
    expect(calculator.calculate('2 * 3')).to eq(6)
  end

  it 'performs division' do
    expect(calculator.calculate('6 / 2')).to eq(3)
  end
end

Good:

F.I.R.S.T (1)

F - Fast

Tests should run quickly because we want to run tests frequently.

I - Independent

Each test should be independent of each other. One test should not be a prerequisite for another test.

R - Repeatable
Tests should be repeatable in any environment (dev, staging, production) and return the same result.

F.I.R.S.T (2)

S - Self validating

Tests should have boolean value: pass or fail. We should be able to tell the result of tests (which ones pass and which ones fail) without lot of hassle.

T - Timely

Unit tests should be written in timely fashion: just before writing the code that will make them pass.

Classes

Classes Should be Small

As with functions and methods, classes should be small. But how small is small?

Single Responsibility Principle (1)

Continuing from the previous point, the ideal "small" size of a class is: only one responsibility for each class.

The Single Responsibility Principle states that a class or module should have one, and only one, reason to change.

Single Responsibility Principle (2)

class Reporter
  def send_report
    users = User.where("last_logged_in_at >= ?", 1.week.ago)

    users.each do |user|
      message = "Id: #{user.id}\n"
      message += "Username: #{user.username}\n"
      message += "Last Login: #{user.last_logged_in_at.strftime("%D")}\n"
      message += "\n"
    end

    Mail.deliver do
      from "jjbohn@gmail.com"
      to "jill@example.com"
      subject "Your report"
      body message
    end
  end
end

Bad:

Single Responsibility Principle (3)

class ReportDataCsvCompiler
  attr_accessor :data
  def initialize(data)
    self.data = Array(data)
  end

  def format
    heading << body
  end

  private

  def heading
    data.first.keys.join(",") << "\n"
  end

  def body
    data.each_with_object("") do |str, row|
      str << row.values.join(",") << "\n"
    end
  end
end

Good - Part 1:

Single Responsibility Principle (4)

class User
  scope :logged_in_this_week, ->{ where("last_logged_in_at >= ?", 1.week.ago) }
end

class UserWeeklyReport
  def self.name
    "Weekly User Report"
  end

  def self.formatter
    ReportDataCsvCompiler
  end

  def self.data
    UserWeeklyReportData
  end

  def to_file
    File.write("/tmp/my_report")
  end
end

Good - Part 2:

Single Responsibility Principle (5)

class ReportMailer
  attr_accessor :report, :recipient

  def intiialize(report:, recipient:)
    self.report = report
    self.recipient = recipient
  end

  def deliver!
    mail = Mail.new do
      from "jjbohn@gmail.com"
      to recipient
      subject report.name
    end

    mail.add_file(report.to_file)
    mail.deliver!
  end
end

Good - Part 3:

Single Responsibility Principle (6)

class UserWeeklyReportData
  def to_data
    User.logged_in_this_week.map do |user|
      {
        id: user.id,
        username: user.username,
        last_logged_in_at: user.last_logged_in_at.strftime("%D"),
      }
    end
  end
end

# Then in you client code
ReportMailer.new(UserWeeklyReport, "jill@example.com")

Good - Part 4:

Important to note that the SRP does mean all classes should have only one function/method. SRP, as the name clearly describes, means all classes should have only one responsibility.

Clean Code

By qblfrb

Made with Slides.com

Clean Code

  • 281

More from qblfrb

  • Insinyur Online - Belajar Ruby - Bagian 2: Variables, Conditionals, dan Boolean Logic
    qblfrb
    189
  • Array in Ruby - with Solution
    qblfrb
    213