S.O.L.I.D in Ruby

Posted by Marcy on November 26, 2018

“Ruby is the best object-oriented language.. if you use it in the right way”

For a long time working in the Ruby world, primarily working on Ruby on Rails projects, I found many projects do not follow proper Object Oriented programming practices. Ruby, as one of the most complete OOP language, provides tons of useful features to implement OOP. In this article, I will demonstrate how to write Ruby code comply with S.O.L.I.D. And some of the common anti-patterns those violate these principles in Ruby on Rails projects.

What is S.O.L.I.D

One the most well-known sets of OO design principles are known by an acronym, SOLID. It stands for:

  • Single responsibility principle (SRP)
  • Open/closed principle (OCP)
  • Liskov substitution principle (LSP)
  • Interface segregation principle (ISP)
  • Dependency inversion principle (DIP)

Let’s take a look at each of them individually, and how you can use them to increase your Ruby code’s quality.

Single responsibility principle (SRP)

“A class should have one, and only one, reason to change.”

Let’s interpret this a little bit. This also means when there is a requirement change, the class should only make the change for the responsibility it holds (so it only has one reason to change).

Example

Let’s look at the following example:

class UserAuthenticator
  def authenticate(email, password)
    if matches?(email, password)
     do_some_authentication
    else
      raise NotAllowedError
    end
  end

  private
  def matches?(email, password)
    user = find_from_db(:user, email)
    user.encrypted_password == encrypt(password)
  end
end

The AuthenticatesUser class is responsible for authenticating the user as well as knowing if the email and password match the ones in the database. It has two responsibilities, and according to the principle it should only have one. Let’s extract one:

class UserAuthenticator
  def authenticate(email, password)
    if PasswordMatcher.new(email, password).matches?
     do_some_authentication
    else
      raise NotAllowedError
    end
  end
end

class PasswordMatcher
  def initialize(email, password)
     @email = email
     @password = password
  end

  def matches?
    user = find_from_db(:user, @email)
    user.encrypted_password == encrypt(@password)
  end
end

Why

This makes change much harder!

  • The more responsibilities a class has, the more likely we need to change the code in the class when there’s requirement change.
  • Code for one responsibility may couple to other responsibilities. Changing code for one feature may break other feature.

Anti-pattern: Fat Model

One of the most common anti-patterns in Rails is Fat model. In a lot of projects, I’ve seen developers implement business logic in the model class.

class Account < ApplicationModel
  after_create :send_notification_email
  include ReportExportable # exports all accounts
  def signin ... end
  def register ... end
  def add_user(user) ... end
  def cancel ... end
end

The model then has a lot of responsibilities

  • Holds data of the model record
  • Handles database operations provided by ActiveRecord
  • All different business logic (with methods, callbacks and concerns)

A fat model class could easily be expanded to hundreds of lines.

What we should do is to

  • Use model classes as data holders and use service classes for bussiness logic
  • Don’t use controllers to handle business logic as it’s only responsible for accepting request and returning response
  • Concerns in models should only contain helpers for accessing its data.
  • Callbacks should only be used for manipulating it’s own data instead of doing any extra work.
class Account < ApplicationModel
end

class SignupService
  def signup(account)
    ...
    SomeEmailSerive.send_notification_email
  end
end

def UserService
  def add_user(account, user) ... end
end

def ReportService
  def export(records) ... end
end

Anti-Pattern: Wrong way of extracting a method

We have a class ContactPolicy which extends AccountResourcePolicy

class AccountResourcePolicy
  def show?
    user.account == resource.account
  end
end

class ContactPolicy < AccountResourcePolicy
  def show?
    return false if contact.organization != user.organization
    super
  end
end

And now we have another policy DocumentPolicy which also extends AccountResourcePolicy

class ContactPolicy < AccountResourcePolicy
  def show?
    return false if contact.organization != user.organization
    super
  end
end

Ok now the developer may think maybe we should extract the duplicate code to a method for matching resource organization to the user’s organization

class AccountResourcePolicy
  def show?
    user.account == resource.account
  end

  protected
  def organization_matched?
    return false if contact.organization == user.organization
  end
end

class ContactPolicy < AccountResourcePolicy
  def show?
    return false unless organization_matched?
    super
  end
end

class DocumentPolicy < AccountResourcePolicy
  def show?
    return false unless organization_matched?
    super
  end
end

Extracting the common logic into a method does improve code reusability. However, the method should not be put into the base class as it is adding a responsibility which the base class should not be responsible for. A better approach is to create another class to handle the responsibility.

class AccountResourcePolicy
  def show?
    user.account == resource.account
  end
end

class OrganizationMatcher
  def self.match?(user, resource)
    user.organization == resource.organization
  end
end

class ContactPolicy < AccountResourcePolicy
  def show?
    return false unless OrganizationMatcher.match?(user, resource)
    super
  end
end

class DocumentPolicy < AccountResourcePolicy
  def show?
    return false unless OrganizationMatcher.match?(user, resource)
    super
  end
end

Open/closed principle (OCP)

“You should be able to extend a classes behavior, without modifying it.”

How can something be open and closed?! A class follows the OCP if it fulfills these two criteria:

  • Open for extension

This ensures that the class behavior can be extended (by inheritance or composition). As requirements change, we should be able to make a new class behave in new and different ways by using the original class.

  • Closed for modification

The source code of such a class is set in stone, no one is allowed to make changes to the code so that we don’t need to change the code that uses this class.

Example

class Report
  def body
     generate_reporty_stuff
  end

  def print
     body.to_json
  end
end

This code violates OCP, because if we want to change the format the report gets printed, you need to change the code of the class. Let’s change it then.

class Report
  def body
     generate_reporty_stuff
  end

  def print(formatter: JSONFormatter.new)
     formatter.format body
  end
end

Why

This makes change much harder!

  • Changing source code of exisitng classes may break clients those use this
  • Adding new code becomes harder as there is no way to reuse existing code

How

  • Make sure your code comply with SRP
  • Anticipate where the change might happen. Make sure you class could adapt to the change (This is a technique for everything in software development…)

Liskov Substitution Principle (LSP)

This one is complicated so let’s talk about this in the end.

Interface Segregation Principle

“when a client depends upon a class that contains interfaces that the client does not use, but that other clients do use, then that client will be affected by the changes that those other clients force upon the class”

This one is simpler to demonstrate, if you have a class that has two clients (objects using it):

class Car
  def open
  end

  def start_engine
  end

   def change_engine
   end
end

class Driver
  def drive
    @car.open
    @car.start_engine
  end
end

class Mechanic
  def do_stuff
    @car.change_engine
  end
end

As you can see, our Car class has an interface that’s used partially by both the Driver and the Mechanic. We can improve our interface like so:

class Car
  attr_reader :control, :internals
end

class CarControl
  def open
  end
  def start_engine
  end
end

class CarInternals
  def change_engine
  end
  def change_wheels
  end
end

class Driver
  def drive
    @car_control.open
    @car_control.start_engine
  end
end

class Mechanic
  def do_stuff
    @car_internals.change_engine
  end
end

By splitting the interface into two, we can comply to the ISP.

Dependency Inversion (DI)

“Abstractions should not depend upon details. Details should depend upon abstractions.”

Let’s go back to the first example on the OCP and change it a bit:

class Report
  def body
     generate_reporty_stuff
  end

  def print
    JSONFormatter.new.format(body)
  end
end

Let’s look at this example in deep. The print method clearly depends on the concrete class JSONFormatter. And the format it prints the report becomes dependent on how JSONFormatter prints the report.

To make this flexible, we allow the print method to accept a formatter. The Report class does not control the format anymore, but only provides the data of the report. And it does not need to know what type of formatter will be passed in to format the data. The only thing it needs to know is that the fomatter object accepts the format method. We call this programming to interface.

class Report
  def body
     generate_reporty_stuff
  end

  def print(formatter)
    formatter.format body
  end
end

This simple example introduces an important technique in OOP - Dependency Inversion. Dependency inversion is relying on abstraction and programming to interface. In the above code, the Report class depends on a formatter object that accepts a format message, instead of a concrete formatter class. We could see the formatter object that accepts a format message as an interface that any concrete formatter class needs to implement. In Ruby, we don’t have a concept of interface like java, but we could simulate the concept like this.

class FormatterInterface
  def format
    raise NotImplementedError
  end
end

class JSONFormatter < FormatterInterface
  def format(data)
    # format data as json
  end
end

The FormatterInterface class is an abstraction of the JSONFormatter. And our report class is dependent on the abstraction instead of the concrete class, which makes changing the report format much more easily.

Liskov Substitution Principle (LSP)

“Derived classes must be substitutable for their base classes.”

You may think this is easy, because we only need to make sure that the derived class is really a base class. But this is not sufficient. LSP is not only for class level but also for method level. Which means if you want to create a derived class, you need to make sure all the methods in the base class still works as expected in the derived class.

Here is an example:

class Shape
end
class Rectangle < Shape
  def set_height ... end
  def set_width ... end
end
class Square < Rectangle
  def set_length ... end
end

Theoretically a square is indeed a rectangle, right? But when we implement this in code it may not be the case. A rectangle may accepts two messages set_width and set_height which allows the client to set the width and height of the rectangle with different lengths. However, for the Square class, it should only accept a set_length method. The set_width and set_height methods are not needed for the square. If we are forced to implement the two methods for the class, whenever we call set_width of a square, we have to to set its ‘height’ to make sure its width and height has the same length. In this case, we can clearly see that a Square class should not be extending the Rectangle class, because the behavior of the inherited methods differs.

Why

This breaks things!

  • As the object of your new class may be used as an object of the existing classes and behave in an unexpected way.

How

  • Follow SCP, OCP and ISP for your base class so your class is small, easier to understand and open for extension
  • Don’t make assumption on the methods in the base class if you think the behaviour might change
  • Try to use Composition over Inheritance when we want to improve code reusability.

Anti-Pattern: Wrong way of override a method

When we override a method, remember to check if the method breaks LSP. After the method is override, it should still have the expected behaviour of the method in the base class.

Let’s review Adapter#pull_and_handle_orphaned_records, SyncGodBase#pull_and_handle_orphaned_records!.

class Adapter
  def full_sync!
    ...
    ...
    pull_and_handle_orphaned_records(...)
  end

  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    ...
    orphan_retry_ids.each do |adapters_resource_id|
      self.delegate_pull!(type_singular, AdaptersResource.find(adapters_resource_id))
    end
    # ...
    mark_missing_records_as_orphaned!(...)
  end
end

class SyncGodBase < Adapter
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    # ...
    orphan_adapters_resources.find_each do |adapters_resource|
      adapters_resource.resource.try(:force_destroy) if adapters_resource.resource_id
      adapters_resource.destroy
    end
  end
end

Some Real World examples

Adapter#pull_and_handle_orphaned_records

Before we move to the actual refactoring, let’s think about why the adapter class is bad (We know it is bad) and why we end up with the classes.

The class was created many years ago, and when it is implemented, it probably looks like this:

class Adapter
  def pull_organizations! ... end
  def pull_locations! ... end
  def pull_contacts! ... end
  def pull_configurations! ... end

  def full_sync!
    ...
    pull_organizations!
    pull_locations!
    pull_configurations!
    ...

    # some logic to handle orphaned records
  end
end

First, this obviously is a fat model that violates the SRP. We would like to turn this into a service at the very beginnning.

class AdapterService
  def pull_organizations! ... end
  def pull...

  def full_sync!
    ...
    pull_organizations!
    ...

    # some logic to handle orphaned records
  end
end

Now we need some other integrations implemented, but they are provided by Sync God so we want to abstract a SyncGodBaseService will be a good idea. And the developer thought SyncGodBaseService should extend the AdapterService class.

class SyncGodBaseService < AdapterService
  ...
end

So SyncGod#full_sync! follows most of the logic in the full_sync! method. However, it handles orphaned records a bit differently. So the developer decided to extract the logic for handling orphaned records in AdapterService to a new pull_and_handle_orphaned_records method. and then let SyncGodBaseService overrides the method. Now the method in the Adapter class marks adapter resources as orphan but the overriden method in SyncGodBase deletes the adapter resource.

class AdapterService
  def full_sync!
    ...
    ...
    pull_and_handle_orphaned_records(...)
  end

  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    ...
    orphan_retry_ids.each do |adapters_resource_id|
      self.delegate_pull!(type_singular, AdaptersResource.find(adapters_resource_id))
    end
    # ...
    mark_missing_records_as_orphaned!(...)
  end
end

class SyncGodBaseService < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    # ...
    orphan_adapters_resources.find_each do |adapters_resource|
      adapters_resource.resource.try(:force_destroy) if adapters_resource.resource_id
      adapters_resource.destroy
    end
  end
end

What is the issue of this approach. As we could see the behaviour of the two methods are almost totally different. This indicates the two classes violate the Liskov substitution principle (LSP). Either the SyncGodBase is not an Adapter or the Adapter class has implemented too much for a base class.

The problem here is in the AdapterService, it makes the assumption that orphaned records are handled by marking all the records as orphaned, while some other AdapterService may handle this in a different ways. Be caution when making assumption in the base class. Because when making assumption, you are creating a contract and the sub class may break the contract after overriding the methods.

To resolve this, we should not make the assumption in a base class. Here is a better approach:

class AdapterService
  def full_sync!
    ...
    ...
    pull_and_handle_orphaned_records(...)
  end

  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    raise NotImplementedError
  end
end

class AdapterServiceA < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    ...
    orphan_retry_ids.each do |adapters_resource_id|
      self.delegate_pull!(type_singular, AdaptersResource.find(adapters_resource_id))
    end
    # ...
    mark_missing_records_as_orphaned!(...)
  end
end

class SyncGodBaseService < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    # ...
    orphan_adapters_resources.find_each do |adapters_resource|
      adapters_resource.resource.try(:force_destroy) if adapters_resource.resource_id
      adapters_resource.destroy
    end
  end
end

Here we have used a design pattern called Template Pattern. Now these classes do not violate the Liskov substitution principle (LSP).

But here is another issue, if we want to implement a AdapterServiceB, we will need to implement the #pull_and_handle_orphaned_records exactly the same to the one in AdapterServiceA. How could we reduce the duplicate codes accross differen classes.

The root problem is that AdapterServiceA and AdapterServiceB violate the Single Responsibility Principle (SRP). The adapter classes should not know how to find orphaned records and how to mark them as orphaned. What we could do is to extract the logics to some OrphanedRecordHandler classes.

class OrphanedRecordHandler
  def handle!(type, synced_adapters_resource_ids)
    raise NotImplementedError
  end
end

class RetryAndMarkAsOrphanedRecordHandler < OrphanedRecordHandler
  def handle!(type, synced_adapters_resource_ids)
    # retry and mark orphaned records as orphaned
  end
end

class DeleteOrphanedRecordHandler < OrphanedRecordHandler
  def handle!(type, synced_adapters_resource_ids)
    # delete orphaned records
  end
end

class AdapterServiceA < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    RetryAndMarkAsOrphanedRecordHandler.new.handle!(type, synced_adapters_resource_ids)
  end
end

class AdapterServiceB < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    RetryAndMarkAsOrphanedRecordHandler.new.handle!(type, synced_adapters_resource_ids)
  end
end

class SyncGodBaseAdapterService < AdapterService
  def pull_and_handle_orphaned_records!(type, synced_adapters_resource_ids)
    DeleteOrphanedRecordHandler.new.handle!(type, synced_adapters_resource_ids)
  end
end

This example introduces another important design pattern which is the Strategy Pattern. Strategy Pattern uses abstraction to encaupsulate different algorithms.