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.