167 Software design basics [4d]

Code that works is not enough; it must stay easy to change. This lesson introduces the principles of maintainable code: avoiding duplication, small responsibilities, clear names and loose coupling.

Important

Work on this lesson in teacher mode.

Learning goals

• You can explain why working code isn't enough: software entropy, and the cost of change over an application's lifetime.
• You can recognize duplication and remove it — and tell when two similar pieces of code are not duplication and should stay apart.
• You can give a class or method a single, clear responsibility.
• You can make code explain itself through good names, and write comments only for the why a name can't express.
• You can explain coupling, the Law of Demeter and "Tell, Don't Ask", and recognize a long chain of calls through other objects as a problem.
• You can design a small public interface that is easy to call from controllers and views, even if that adds complexity inside the class.
• You can put logic where it belongs and keep concerns apart that tend to get tangled — e.g. calculations out of views into models, searching separate from authorization — so each part can change on its own.
• You can explain YAGNI and resist building for requirements that don't exist yet.
• You can judge a piece of code — yours, a colleague's or an agent's — against these principles and say what you would change and why.

Resources

Read what's new to you, skim what's familiar, skip what you already master. Stop when you can meet the learning goals.

Your agent can also generate an overview, a tutorial or an explanation for anything here, tailored to what you already know. Just ask.

Books (in our library)

• 📘 The Pragmatic Programmer, anniversary edition — Topic 3 "Software Entropy", Topic 9 "The Evils of Duplication", Topic 10 "Orthogonality", Topic 28 "Decoupling" (incl. the Law of Demeter)
• 📘 Practical Object-Oriented Design in Ruby (Sandi Metz) — chapters 3 "Managing Dependencies", 5 "Reducing Costs with Duck Typing", 6 "Acquiring Behavior Through Inheritance", 7 "Sharing Role Behavior with Modules"; code at poodr2 Show archive.org snapshot
• 📘 Clean Code — chapters 2 "Meaningful Names", 3 "Functions", 4 "Comments" and 17 "Smells and Heuristics"
• 📘 Growing Rails Applications in Practice — chapters 5 "Dealing with fat models" and 7 "Extracting service objects"

Articles

• 📄 How to write modular code — our card
• 📄 Keep It DRY, Shy, and Tell the Other Guy Show archive.org snapshot — Hunt and Thomas, PDF
• 📄 Tell, Don't Ask Show archive.org snapshot
• 📄 GRASP (object-oriented design) Show archive.org snapshot — nine principles for assigning responsibilities to classes
• 📄 Yagni Show archive.org snapshot — Martin Fowler; and the counterpoint YAGNI exceptions Show archive.org snapshot by Luke Plant
• 📄 Best practices for writing code comments Show archive.org snapshot

Video

• ▶️ All the Little Things Show archive.org snapshot — Sandi Metz, RailsConf 2014, 40 min: refactoring a nested conditional into small objects, live

Discuss with your mentor what you took away from each topic.

Rules of thumb

Apart from the concepts mentioned above, there are a many more rules that are good to follow most of the time. For example:

• Embrace Locality: Avoid cluttering one concept across the entire code base. It should be encapsulated in single files or folders.
• Be easy to call: A service should always try to have a simple public interface, even if that causes additional complexity within the class. For example:
  • Interacting with the model should be easy for controllers and views
  • Interacting with helpers and routes should be easy for views
• Avoid long instruction manuals: If you need to write down an A4-letter for your colleagues on how to use your service, there might be a way to refactor it to a simpler public interface. A post-it sized note should be enough, if anything!

Exercises

Invoice view

We're building an e-commerce app where users can create and view invoices.

This is our current model:

class Invoice < ApplicationRecord
  has_many :items
  validates_presence_of :recipient_address, :number
end

class Invoice::Item < ApplicationRecord
  belongs_to :invoice
  belongs_to :product
  validates_numericality_of :units
end  

class Product < ApplicationRecord
  validates_presence_of :description
  validates_numericality_of :unit_price
end

This is a view that shows an invoice:

%h1
  Invoice
  = @invoice.number

%h2 Recipient
= @invoice.recipient_address

%h2 Items

%table
  %tr
    %th Description
    %th Quantity
    %th Item total

  - @invoice.items.each do |item|
    %td= item.product.description
    %td= item.units
    %td= item.units * item.product.unit_price
    
  %tr
    %th
      Invoice total
    %td(colspan=3)
      = @invoice.items.sum { |item| item.units * item.product.unit_price } * 1.19

How would you judge the quality of this code? Try to apply what you learned with a refactoring of the model and the view. What are the advantages of your solution?

MovieSearch vs. Authorization

Let's say there is the a MovieSearch class in your project with the following public API:

class MovieSearch
  def initialize(query)
    @query = query
  end
  
  def results
    Movie.where('title LIKE ?', @query)
  end
end

search = MovieSearch.new('Interstellar')
search.results.each do |movie|
  puts movie.title
end

Now the current user should not be allowed to search all movies, but only a subset based on their role and the movie's state. A naive inline authorization could look like this:

class MovieSearch
  def initialize(query, current_user)
    @query = query
    @current_user = current_user
  end
  
  def results
    scope = Movie.where('title LIKE ?', @query)
    unless current_user.moderator?
      scope = scope.where('state = "approved" OR user_id = ?', @current_user.id)
    end
    scope
  end
end

Try to come up with an alternative implementation where the two concepts (movie search and authorization) are less coupled.