One of the most common best practices in Rails is to keep the controller light weight. This helps maintain the code’s readability maintainability, and refactorability (just like coding, I make stuff up). This is often done by pushing as much of the business logic as you can (where it still makes sense) down into a model.

At kaChing, we don’t use ActiveModel as our backend layer. Instead, persistence is handled over the wire through RPC requests. However developers often forget (including myself) that models themselves are good encapsulation objects and can be used beyond the ActiveModel layer. Just the other day, I started out by writing the following code sample in a controller method

def some_method

 date_string = nil

 if params[:param_a] == "false"
  begin
   date_string = params[:date].to_date.strftime("%Y%m%d")
  rescue
   raise "Some error"
  end
 end
 
 array = []
 for key, value in params[:param_C]
  # initialize array
 end

 if params[:param_b] == "set"
  make_rpc_request(params[:account_id]
 end

 make_another_rpc_request(params[:account_id], array, date_string)
end

There are some problems with the above code beyond the bloated controller method
a) it makes testing this controller method harder because now it requires the developer to know how to setup the test, which leads to…
b) testing becomes more fragile.

What I ended up doing instead was to refactor the controller by moving the business logic into a param model as follows

class SomeMethodParams
 attr_reader :array, :param_b_is_set, :account_id, :date
 
 def initialize(params)
    # init params here
    @param_a = params[:param_a] == "true" ? true : false
    @param_b_is_set = params[:param_b] == "set" ? true : false
    @param_c = params[:param_c]
    @account_id = params[:account_id]
    @date = params[:date]
    initialize_date(params)
    initialize_array(params)
 end

 def initalize_date(params)
  unless @param_a
   @date_string = @date.to_date.strftime("%Y%m%d")
  end
 rescue
  raise "Some error"
 end

 def initalize_array(params)
  for key, value in @param_c
   # set array
  end
 end
end

I can then simplify and refactor the controller code down to

def some_method
 some_method_params = SomeMethodParams.new(params)
 make_rpc_request(some_method_params.account_id) if some_method_params.param_b_is_set
 make_another_rpc_request(some_method_params.account_id, some_method_params.array, some_method_params.date_string)
end

What’s great now is that not only do I have a super lightweight controller method,
I also have a couple of other additional benefits:
1) the code is easier to read (keep other developers happy)
2) testing becomes easier, less fragile, and more focused (separation of concerns)