Search code examples
ruby-on-rails-3refactoringdelegation

Delegate or instantiate additional class?


Let's say I have an Account class and an AccountReport class. In accounts#show I want to show a report of an account. Both Account and AccountReport have a number of public methods. Which technique of the following techniques is better?

1) Instantiate an Account and an AccountReport, initializing the AccountReport with the account data.

class AccountsController < ActionController::Base
  def show
    @account = current_user.account
    @account_report = AccountReport.new(@account.orders)

    respond_with(@account)
  end

  # ...
end

2) Allow an instance of Account to instantiate AccountReport and delegate method calls

class Account < ActiveRecord::Base
  attr_reader :account_report

  delegate :method_a, :method_b, :method_c, :method_d, :to => :account_report

  after_initialize :setup_account_report

  def setup_account_report
    @account_report = AccountReport.new(orders)
  end

  # ...
end

Option 2 seems to be a cleaner approach to me but loading up Account with lots of methods makes it feel like a God class.


Solution

  • Well, i think you have to make a mix of both option.

    The first one is good, if you only use reports on show. The second one is good, if you use all the time reports for your account.

    With the second one, all the time your report will be instantiate and it could reduce performances.

    You should perhaps try something like this:

    class Account < ActiveRecord::Base
      # ...
    
      @report = nil
      def report
        if @report.nil?
           @report = AccountReport.new(self.orders)
        end
      end
    
      # ...
    end
    

    The good thing of this solution is that report is loaded only if needed, but will not be loaded every time. The bad thing of this solution is that if you add some orders your report will not be up to date.

    UPDATE: To improve this, you could replace the condition with this one

    if @report.nil || self.created_at_changed?