# Similar methods to update today's sales, revenue, and profit

Please help me to refactor. I have two models, conversion & statistic. Methods sales_today, revenue_today, profit_today in conversion model only gets specific info from db.

Statistic model

class Statistic < ActiveRecord::Base

class << self

def create_and_update_today(agregate_name, value, network)
today = find_or_create_by(agregateName: agregate_name, networkName: network)
today.update(agregateName: agregate_name, date: Date.today,
value: value, networkName: network)
end

def sales_today(network)
total = Conversion.sales_today(network)
create_and_update_today('salesToday', total, network)
end

def revenue_today(network)
total = Conversion.revenue_today(network)
create_and_update_today('revenueToday', total, network)
end

def profit_today(network)
total = Conversion.profit_today(network)
create_and_update_today('profitToday', total, network)
end

end
end


Conversion model

class Conversion < ActiveRecord::Base

class << self

def sales_today(network)
where('DATE(created_at)= ?  AND networkName = ?', Date.today, network).size
end
etc...

end


Is there any way to refactor almost similar pieces of code?

• I'm not sure I understand: which class is this code excerpt from? Could you post the Conversion class as well? – 200_success Jan 28 '15 at 11:25

Instead of dynamically defining a bunch of methods, you could just have one method that takes an argument that tells it what kind of work to do:

def self.create_or_update_today(kind, network)
meth = :"#{kind}_today"
total = Conversion.send(meth, network)

aggregate_name = "#{kind}Today"

today = find_or_initialize_by(agregateName: aggregate_name, networkName: network)
today.update(date: Date.today, value: total)
end


You would call it like this:

Statistic.create_or_update_today(:sales, network)


(You'll notice I used find_or_initialize_by instead of find_or_create_by. This way the code will always perform two queries instead of performing three if it's a new record. update will create the record of it doesn't exist.)

If you wanted you could still define individual methods, but this way it's much cleaner:

def self.sales_today(network)
self.create_or_update_today(:sales, network)
end

def self.revenue_today(network)
self.create_or_update_today(:revenue, network)
end

# ...


You could also do this with define_method in a loop as Devon does, but it would save very few lines while sacrificing a lot of readability and, as britishtea points out, introducing documentation issues.

P.S. Just a tip. Don't use size for this:

where('DATE(created_at) = ?  AND networkName = ?',
Date.today, network).size


You only want to know how many records there are, but when you do this it fetches all of the data in all of the records and initializes a Conversion object for each, which is really wasteful. Instead of size, use Rails' built-in count method:

where('DATE(created_at) = ?  AND networkName = ?',
Date.today, network).count


This will let the database perform the calculation so ActiveRecord only needs to fetch a single number.

In addition to Devon's answer. You can put method definitions right in the body of class:

class Statistic < ActiveRecord::Base

class << self
[:sales_today, :revenue_today, :profit_today].each do |method_name|
define_method method_name do |network|
total = Conversion.send(method_name, network)
create_and_update_today(method_name.to_s.camelcase(:lower), total, network)
end
end

def create_and_update_today(agregate_name, value, network)
today = find_or_create_by(agregateName: agregate_name, networkName: network)
today.update(date: Date.today, value: value)
end
end
end


Also I've removed aggregate and network names from #update call, as they won't update anyway.

You can do some nifty meta-programming to dynamically define the methods.

The code below is slightly modified to show the results, you should find it trivial to change back to suit your needs.

class Foo < ActiveRecord::Base
def initialize
super
[:sales_today,:revenue_today,:profit_today].each do |m|
self.class.send(:define_method, m) do |network|
total = 2 # Conversion.sales_today(network)
# create_and_update_today(m, total, network)
do_work(m, total, network)
end
end
end

def do_work (one, two, three)
puts "#{one} #{two} #{three}"
end
end

foo = Foo.new

puts "Methods: #{foo.methods}"
foo.sales_today("network 1")
foo.revenue_today("network 2")
foo.profit_today("network 3")


Output:

Methods: do_work
sales_today
revenue_today
profit_today
nil?
===


...

sales_today 2 network 1
revenue_today 2 network 2
profit_today 2 network 3

• Won't this redefine those class methods every time a new instance of Foo is initialized? – Jordan Jan 29 '15 at 6:52
• @Jordan Hmm, I think it will, let me look into this further to see if I can prevent that – Devon Parsons Jan 29 '15 at 9:34
• Keep in mind that defining methods dynamically like this, keeps them out of your documentation. You might prefer a little duplication over methods hidden in documentation. – britishtea Jan 29 '15 at 10:40