Refactoring Rails: Strategies to refactor models

There’s abundant online guidance about refactoring controllers and keeping them lightweight, but resources on model refactoring are less common. In this blog, we’ll explore some techniques for effectively refactoring models.

The refactoring goal remains consistent whether you’re working on a controller, model, or any other file. The current file might be complex, lengthy, or error-prone. Our objective is to create code that is:

1. Easier to understand
2. Simpler to maintain
3. Less susceptible to bugs

Before beginning the refactoring process, two critical prerequisites must be addressed:

1. How do you determine that a file needs refactoring?
2. Do you have sufficient test coverage for the file?

How do you determine that a file needs refactoring?

At FastRuby.io opens a new window , we rely on two primary methods to identify files that need refactoring:

1. Static Code Analysis Tools

We use tools like RubyCritic opens a new window and Skunk opens a new window to evaluate code complexity and churn opens a new window . These tools provide insights into:

• The complexity of a file
• How frequently the file changes
• Potential areas of technical debt

If these tools highlight a file as problematic, it’s a strong signal that refactoring might be necessary. For example, a file with an ‘F’ rating in RubyCritic due to high churn and complexity might be a candidate for refactoring.

2. Team Consensus and Practical Observations

Sometimes, the need for refactoring becomes apparent through team feedback or customers feedback. This typically happens when:

• Most engineers find the file difficult to understand
• Team members are hesitant to make changes
• There’s a fear of unintentionally breaking critical business logic

Customers might also complain that they often face issues using certain part of the application, which can be an indication that there could be a hidden bug or unnecessary complexity.

Interestingly, these practical concerns often correlate with the metrics from static code analysis tools. The team’s collective experience is often as valuable as automated metrics for identifying files that need attention.

Is there enough test coverage for this file?

What constitutes sufficient test coverage? At FastRuby.io, we typically consider 90% test coverage for the file to be refactored as the minimum threshold before beginning the refactoring process. This high level of test coverage is crucial and serves as the foundational first step in our refactoring journey. By ensuring comprehensive test coverage, we accomplish several key objectives:

• Validate the existing functionality of the code
• Create a safety net that catches potential regressions
• Provide confidence when making structural changes

Our 90% coverage benchmark means that before we touch a single line of code, we want to be certain that we’ve captured and verified most of the file’s current behavior. This approach minimizes the risk of introducing unexpected bugs during the refactoring process. And in case the test coverage is not enough, then we make a point to write tests before starting any refactoring work.

While having sufficient test coverage is crucial, it is equally important to ensure the quality of the tests. For example, if we write a test for an API and only assert the response code without validating the response body, we cannot fully rely on that test because it only verifies a portion of the functionality. High-quality tests are just as important as coverage, and we make it a priority to analyze and improve the quality of tests as the first step in the refactoring process. This analysis is a manual process where we review tests and closely examine their assertions to ensure they provide comprehensive validation. And we make it a point to analyse the tests for quality and improve if needed as the first step of refactoring. The process of analysing the quality of tests is manual i.e we go through tests and observe the assertions.

Strategies for Refactoring a Rails Model

Methods with Same Prefixes

When refactoring a model, one effective technique is to identify methods with similar prefixes. This can reveal opportunities for creating more organized, focused code. Let’s walk through an example: Imagine a model with methods like:

• def attend_biology
• def attend_physics
• def attend_sem_break

These methods share the attend_ prefix, suggesting a potential for extracting a common abstraction. In this case, we can create a new class called AttendEvent:

class AttendEvent
  def initialize(student)
    @student = student
  end

  def physics
    # Specific logic for attending physics event
  end

  def biology
    # Specific logic for attending biology event
  end

  def sem_break
    # Specific logic for attending sem_break event
  end
end

Before refactoring, you might have code like this:

student.attend_biology

After refactoring, it would look like:

AttendEvent.new(student).biology

Benefits of this approach:

• Encapsulates event-related logic in a dedicated class
• Establishes a clear location for adding new events
• Improves code organization and readability
• Makes the codebase more maintainable

An important aspect of refactoring is making incremental changes over time rather than attempting a massive transformation. This incremental approach:

• Simplifies code review processes
• Reduces the risk of introducing bugs
• Makes testing more straightforward
• Allows for easier rollback if needed

It is also important to observe that the methods as a group should belong together, and not forced into a class just because they have the same prefixes.

Complex Callbacks Business Logic

In complex Rails models, business logic often gets tangled within before_* and after_* callbacks, creating cluttered and hard-to-read code. Let’s explore a strategy to clean up these callbacks and improve code organization. Consider a typical scenario where email notifications are managed directly in model callbacks:

class Student < ApplicationRecord

  before_save do |s|
    if s.registered?
      EmailNotification.send_registration_confirmation_email(s)
    elsif s.fees_paid?
      EmailNotification.send_fees_received_email(s)
    else
      EmailNotification.send_registration_reminder_email(s)
    end
  end

  after_create do |s|
    if s.schedule_selected?
      EmailNotification.send_schedule_confirmation_email(s)
    elsif s.major_selected?
      EmailNotification.send_major_confirmation_email(s)
    end
  end
end

This approach has several drawbacks:

• Obscures the model’s primary purpose
• Creates visual “noise”
• Makes the code difficult to read and understand
• Mixes business logic with model definition

Refactoring with a Callback Policy By introducing a StudentCallbackPolicy class, we can dramatically improve code clarity:

class Student < ApplicationRecord

  before_save do |s|
    StudentCallbackPolicy.new(s).before_create
  end

  after_create do |s|
    StudentCallbackPolicy.new(s).after_create
  end
end

The corresponding StudentCallbackPolicy might look like:

class StudentCallbackPolicy
  def initialize(student)
    @student = student
  end

  def before_create
    handle_registration_notifications
  end

  def after_create
    handle_confirmation_notifications
  end

  private

  def handle_registration_notifications
    if @student.registered?
      EmailNotification.send_registration_confirmation_email(@student)
    elsif @student.fees_paid?
      EmailNotification.send_fees_received_email(@student)
    else
      EmailNotification.send_registration_reminder_email(@student)
    end
  end

  def handle_confirmation_notifications
    if @student.schedule_selected?
      EmailNotification.send_schedule_confirmation_email(@student)
    elsif @student.major_selected?
      EmailNotification.send_major_confirmation_email(@student)
    end
  end
end

Benefits of This Approach

1. Improved Code Organization

• Separates callback logic from the model
• Creates a clear, dedicated location for callback-related business logic
• Makes the model more focused and readable

2. Enhanced Maintainability

• Provides a predictable structure for future callback logic
• Makes it easy to add new notification methods
• Simplifies understanding of what happens during different model lifecycle events

3. Easier Testing

• Allows for straightforward unit testing of callback logic
• Can easily instantiate and test the policy class in isolation

4. Future Flexibility

• When adding new notification channels (like SMS), engineers know exactly where to add the logic
• Promotes a consistent approach to handling model callbacks
• This brings us one step closer to eliminating the use of callbacks for critical business logic, ensuring that such logic is handled in a more explicit and maintainable way.

Practical Considerations

The term “policy” might seem unconventional here, but it represents a class responsible for making decisions about a model’s behavior. Feel free to rename the class to something that feels more intuitive for your team, such as StudentCallbackHandler or StudentNotificationManager. While we do suggest to encapsulate callback logic under a class, it is also important to decide on the scope of the class.

Finding Methods with Similar Purpose

In complex Rails applications, access control logic often accumulates in models, making them bloated and difficult to manage. Let’s explore a strategy to refactor and organize permission-related code.

Initial Scenario: Permission Methods in the Model Consider a typical implementation where permission methods are directly added to the model:

class Course < ApplicationRecord
  def can_approve?(user)
  end

  def can_assign?(user)
  end

  def can_reject?(user)
  end

  def can_duplicate?(user)
  end

  def can_delete?(user)
  end
end

These methods might be used across controllers, views, and helpers like Course.can_assign?(user).

Refactoring Approaches

We recommend two primary strategies for handling access control:

1. Use an Existing Gem

• Evaluate permission management gems that fit your application’s needs
• Examples include Pundit, CanCanCan, or Rolify
• Ideal for comprehensive, application-wide permission restructuring

The pros of taking this approach are:

• Quick setup
• Standardized patterns

The cons of taking this approach are:

• Higher learning curve
• Less flexibility

2. Incremental Refactoring: Access Policy Class

• When a complete gem implementation feels too drastic, an incremental approach can be more manageable.

Proposed Refactoring: Access Policy Class

class Course::AccessPolicy
  def initialize(course, user)
    @course = course
    @user = user
  end

  def can_approve?
    # Implement approval logic based on user roles
  end

  def can_assign?
    # Implement assignment logic
  end

  def can_reject?
    # Implement rejection logic
  end

  def can_duplicate?
    # Implement duplication logic
  end

  def can_delete?
    # Implement deletion logic
  end
end


class Course < ApplicationRecord
  def policy_for(user)
    Course::AccessPolicy.new(self, user)
  end
end

Benefits of This Approach

1. Improved Code Organization

• Removes complex logic from the model
• Creates a dedicated space for permission-related code
• Makes the model more focused and readable

2. Enhanced Maintainability

• Centralizes access control logic
• Easier to modify and extend permissions
• Provides a clear structure for future changes

3. Flexibility in Usage

• Replace course.can_access?(user) with course.policy_for(user).can_access?
• Allows for more granular and explicit permission checks

4. Easier Testing

• Simplifies unit testing of permission logic
• Can test AccessPolicy class in isolation

Example Usage

# Before
if course.can_approve?(current_user)
  # Approve course
end

# After
if course.policy_for(current_user).can_approve?
  # Approve course
end

Practical Considerations

• Start with a single model’s access policy
• Gradually refactor other models using the same pattern
• Consider a gem solution for more complex, application-wide permissions
• Ensure consistent naming and structure across policies

Finding utility methods

In Rails applications, developers often add miscellaneous methods directly to models, even when these methods don’t truly belong to the model’s core responsibilities. Let’s explore a better approach to handling such utility methods.

Initial Scenario: Utility Methods in the Model Consider a typical implementation where currency conversion methods are added to a model:

class Course < ApplicationRecord
  def self.price_in_euro(price)
    price * rate_of_euro_per_usd
  end

  def self.price_in_inr(price)
    price * rate_of_inr_per_usd
  end
end

Usage might look like: Course.price_in_euro(course.price).

Problems with This Approach

• Methods are tightly coupled to the Course model
• It feels unnatural to call currency conversion methods on a course
• It is difficult to reuse these methods across different models

Refactoring: Extracting Utility Methods We can create a dedicated utility class in the app/lib folder:

# app/lib/currency_convertor.rb
module CurrencyConvertor
  class << self
    def convert_to(price, target_currency)
      case target_currency.downcase
      when 'euro'
        price_in_euro(price)
      when 'inr'
        price_in_inr(price)
      else
        raise ArgumentError, "Unsupported currency: #{target_currency}"
      end
    end

    private

    def price_in_euro(price)
      price * rate_of_euro_per_usd
    end

    def price_in_inr(price)
      price * rate_of_inr_per_usd
    end

    def rate_of_euro_per_usd
      # Implement exchange rate retrieval logic
    end

    def rate_of_inr_per_usd
      # Implement exchange rate retrieval logic
    end
  end
end

While I would put this file in the app/lib, this could also be structured as a service object or concern. We generally prefer service objects when the code involves business logic. In this case, however, the focus is more on computation than on business logic, making a utility class in app/lib a suitable choice.

Benefits of the Utility Class Approach

1. Improved Code Organization

• Separates conversion logic from domain models
• Creates a clear, reusable location for utility methods
• Follows the Single Responsibility Principle

2. Enhanced Flexibility

• Can be used across different models and contexts
• Easy to extend with new currency conversions
• Centralized location for currency-related logic

3. Usage Across the Application

# Can be used anywhere in the application
converted_price = CurrencyConvertor.convert_to(course.price, 'euro')

4. Easy to Maintain and Test

• Isolated class makes testing straightforward
• Can easily mock or stub currency conversion logic
• Clear separation of concerns

A special processor class

In Rails applications, it’s common to find business logic methods cluttering model files. While models should primarily focus on data retrieval and basic validations, complex business logic often finds its way into these classes. The main concern with mixing business logic into models is that it makes the models harder to read and understand, while also creating challenges for testing, as the tests need to cover both data concerns and business logic. Let’s explore a more structured approach to handling such logic.

Initial Scenario: Business Logic in the Model

Consider a typical implementation where business actions are defined directly in the model:

class Course < ApplicationRecord
  def approve
    # Complex approval logic
  end

  def assign
    # Complex assignment logic
  end

  def duplicate
    # Complex duplication logic
  end
end

Problems with This Approach

• Makes models bloated and hard to maintain
• Intertwines data representation with business logic.
• Reduces code readability and testability

Refactoring: Introducing a Processor Class We can extract these business logic methods into a dedicated processor class:

class Courses::Processor
  def initialize(course)
    @course = course
  end

  def approve
    # Detailed approval workflow
  end

  def assign
    # Comprehensive assignment logic
  end

  def duplicate
    # Complex duplication process
  end
end

It is important to note that we do not want this class to become a place to dump all methods in. We should avoid bloating this class with unrelated operations and rather extract methods that belong together to a seperate service object.

Benefits of the Processor Class Approach

1. Clear Separation of Concerns

• Removes business logic from the model
• Creates a dedicated space for complex workflows
• Keeps models focused on data representation

2. Improved Code Organization

• Encapsulates related business logic
• Makes code more modular and readable
• Allows for more detailed, step-by-step processes

3. Enhanced Testability

• Easy to write comprehensive tests for each action
• Can mock or stub specific steps in the process
• Isolates business logic for more focused testing

4. Flexible Usage

#Clean, explicit usage
Courses::Processor.new(@course).approve
Courses::Processor.new(@course).assign

Usage Example in a Controller

class CoursesController < ApplicationController
  def approve
    @course = Course.find(params[:id])

    begin
      Courses::Processor.new(@course).approve
      redirect_to @course, notice: 'Course approved successfully'
    rescue StandardError => e
      redirect_to @course, alert: "Approval failed: #{e.message}"
    end
  end
end

Using Reek to Identify Code Smells

Reek is a powerful static code analysis tool for Ruby that helps identify potential design issues and code smells. However, it’s crucial to approach its warnings with a nuanced perspective. It is also the last step in the refactoring journey.

Purpose of Reek Reek scans Ruby code and highlights potential:

• Design issues
• Complexity concerns
• Potential refactoring opportunities
• Code maintainability problems

Key Considerations

1. Not All Warnings Are Absolute

• Reek provides suggestions, not mandatory rules
• Some “code smells” might be intentional design choices
• Team-specific coding patterns may trigger warnings

2. Selective Addressing

• Focus on warnings that genuinely impact code quality. One of my personal favourite is unused variable warning opens a new window . It is easy to store something in a variable as you are writing the code, you may think you need this variable. But as you iron out the details, sometimes variables that are not used in the code lingers around, adding to the noise in the codebase. They are easiest to get rid of, and immediately makes thing little cleaner.

Conclusion

These are some of the factors we consider when refactoring a Rails model. However, we don’t recommend applying these techniques indiscriminately across your application. Refactoring isn’t just about improving the code—it’s also about considering the people who work with it, making it a highly subjective process. While we might favor a particular structure, it’s important to ensure alignment with the team. Ultimately, the goal of refactoring is to maintain a codebase that is easy for everyone to understand, collaborate on, and modify.

I hope this article provides valuable insights and helps you keep the models in your application clean and maintainable.

Need help with refactoring your Rails application? Talk to us today! opens a new window