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.
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?