Skip to content

Instantly share code, notes, and snippets.

@IdahoEv
Last active September 23, 2015 22:22
Show Gist options
  • Save IdahoEv/adc6c6d3e0fa3a115c76 to your computer and use it in GitHub Desktop.
Save IdahoEv/adc6c6d3e0fa3a115c76 to your computer and use it in GitHub Desktop.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment