The code in question looked like this (shortened version):
def count_of_beats_by_kind
set = @project.beats
list = BEAT_KINDS
list.each do |kind|
counts[kind] = set.where(:kind => kind).count.to_f
end
counts["total"] = set.count.to_f
return counts
end
When starting with an empty Enumerable
and iterating something else to append to it, there's usually a shorter way in Ruby that's more idomatic. What I mean is this common pattern:
my_hash = {} # or my_array = []
some_array.each do |thing|
my_hash[thing] = calculation_based_on(thing)
end
return my_hash
When making one Array from another it's fairly obvious: use Array#map
. When making a Hash from something else, there are two less-obvious-but-super-useful approaches to know: the class method Hash.[]
and the ability to collect things into a Hash with Enumerable#inject
or Enumerable#each_with_object
(which are similar and related). I'll let you look up what those two approaches do, but here's how to rewrite this code with them.
counts = Hash[
list.zip( list.map{ |kind| set.where(:kind => kind).count.to_f).flatten
]
On retrospect, I think that one's less readable in this case. But useful to understand how it works. The other is:
counts = list.each_with_object({}){
|hash, kind| hash[kind] = set.where(:kind => kind).count.to_f
}
Which I think is pretty clear if you know what each_with_object does. THAT SAID, while it's important to know the above approaches, in this case none of these approaches are correct for an important reason: they're all performing a database call inside a loop or iterator, with each database call returning only a single number. Database calls are expensive -- a couple milliseconds each no matter how small -- so when you call them in a loop to retrieve one item at a time, you are pretty much guaranteeing a slow website.
Fortunately SQL has the ability to group things by a value and do calculations on each group, returning them all in a single query. If you were writing raw SQL it would look something like this (for project beats in project #5):
SELECT kind, count(*) FROM 'beats' WHERE beats.project_id = 5 GROUP BY beats.kind
That query would give you a bunch of pairs of kind and the counts of beats with that kind. Which is exactly what you're trying to build by hand.
Fortunately ActiveRecord has tools to make that super-easy for you. Look at this reference: http://api.rubyonrails.org/classes/ActiveRecord/Calculations.html#method-i-count and see where it explains using count
and group
together.
With that, you can actually convert the meat of this method to a single line while making it faster in the process! Neat...
One tip: the fact that the Project#beats
has_many association has an order attached to it will cause you trouble if you use the association (See project.rb line 3), because you can't GROUP BY one column while doing ORDER BY another in SQL. The solution is to just use Beat.where(:project_id => project.id) instead of project.beats.