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)