# Establishing parameters for a sales report, with default boolean and date values

The idea is that I am getting the params from a GET request in the form of a Hash (symbol→str) and I want to transform them into dates and booleans. For that reason, I created two private methods to_date and to_b.

class Report
include ActiveModel::Model

attr_accessor :start_date, :end_date, :show_sales, :show_daily_active_users,
:show_registered_cc, :show_all_time_registered_cc, :show_users

def initialize(params)
params ||= {}
@start_date = to_date(params, :start_date, Time.zone.today - 7.days)
@end_date = to_date params, :end_date, Time.zone.today
@show_sales = to_b params, :show_sales, true
@show_daily_active_users = to_b params, :show_daily_active_users, true
@show_registered_cc = to_b params, :show_registered_cc
@show_all_time_registered_cc = to_b params, :show_all_time_registered_cc
@show_users = to_b params, :show_users
end

def sales
(...)
end

(...)

private

def to_date(params, key, default_val)
if params.key?(key)
Date.parse(params[key])
else
default_val
end
end

def to_b(params, key, default_true = false)
default_val = '1' if default_true
params.fetch(key, default_val) == '1'
end
end


The problem is that Rubocop says that the Assignment Branch Condition size for the method initialize is too high (15.81 over a limit of 15).

Is there anything I could change to fix that? I don't think that creating a new class just to do that is worth it.

• Thanks for the edit @Jamal. I was thinking that it could be a good idea to add a metrics and/or abc_metrics tags to this question, but I don't have enough Internet Points – Daniel Feb 10 '16 at 4:09
• The cyclometric-complexity tag covers it. – 200_success Feb 10 '16 at 4:51

You could reduce branching and improve readability by requiring the default for to_b to be explicitly stated. (It seems a bit excessive to have a default for the default, don't you think?)

The branching is not my primary concern, though. The constructor seems to have a very copy-and-paste pattern. Metaprogramming could help here, I think. It would be nice to write…

class Report
include ActiveModel::Model
include ParamAccessors

date_param_accessor(:start_date) { |today| today - 7.days }
date_param_accessor :end_date
bool_param_accessor :show_sales, true
bool_param_accessor :show_daily_active_users, false
bool_param_accessor :show_registered_cc, false
bool_param_accessor :show_all_time_registered_cc, false
bool_param_accessor :show_users, false

def initialize(params)
@params = params
end

def sales
…
end
end


This mixin could make that possible:

module ParamAccessors
def self.included(base)
base.extend ClassMethods
end

module ClassMethods
def date_param_accessor(name, default=nil)
attr_writer(name)
define_method(name) do
if instance_variable_defined?("@#{name}")
instance_variable_get("@#{name}")
else
@params.fetch(name, default) or
(yield Time.zone.today if block_given?) or
return Time.zone.today
end
end
end

def bool_param_accessor(name, default)
attr_writer(name)
define_method(name) do
if instance_variable_defined?("@#{name}")
instance_variable_get("@#{name}")
else
@params.fetch(name, default)
end
end
end
end
end


This assumes that the object is transient, such that the constructor and the start_date/end_date methods will be called on the same day.

• This refactoring looks great! I learned many things. Thanks! Out of curiosity, would you put the ParamAccessors module in the Concerns directory? – Daniel Feb 10 '16 at 5:39
• Also, yes, the model is transient. Its function is just to provide data of several reports to a controller. – Daniel Feb 10 '16 at 5:51
• That seems reasonable. In fact, if you have the module extend ActiveSupport::Concern, then you can write it slightly more gracefully, as in the docs. – 200_success Feb 10 '16 at 5:52
• Sorry, there's something I don't understand. What is the point of the if instance_variable_defined?("@#{name}"); instance_variable_get("@#{name}")? With your solution, the attributes become methods instead of instance variables. – Daniel Feb 10 '16 at 6:07
• Your original code had attr_accessor rather than attr_reader, so I assumed that you wanted those attributes to be writable, which means that when defining the attribute reader, you need to check the instance variable. – 200_success Feb 10 '16 at 6:17