I have this monstrosity

    @nation_biopsy =  
       User.all.where(country: @nation).group_by{ |u| [u.institute_type, u.biopsy] }
      .map{ |i, o| ["name" => i[0], "y" => User.where(id: o.map(&:id))

What it is doing is finding all the users in my database that belong to a country (@nation) and grouping them according to their "institute_type" and then "biopsy" (both columns for users). Then it is trying to convert it to an array of JSON objects for a "Highcharts" script. It works, but as a newbie I am sure this is very database heavy. Is there a better way?

  • 1
    \$\begingroup\$ I'm more concerned that all this information is in a single User table. Tell us more about your database schema? \$\endgroup\$ – 200_success Dec 12 '15 at 17:11
  • \$\begingroup\$ The User has 10 attributes. I'm not sure what you're saying. There are only 3 attributes cited above....biopsy, institute_type and country \$\endgroup\$ – GhostRider Dec 12 '15 at 17:23
  • \$\begingroup\$ This snippet belongs to a model or a controller? \$\endgroup\$ – tokland Dec 12 '15 at 21:04
  • \$\begingroup\$ This is really interesting, look forward to seeing a rewrite. \$\endgroup\$ – 13aal Dec 13 '15 at 14:42
  • \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Vogel612 Dec 13 '15 at 18:27

Some notes on your code:

  • User.all.where -> User.where.
  • The moment you write group_by you are not using SQL anymore, performance will suffer.
  • The line starting with sum is very confusing, the indentation make you think it's the continuation of map but in fact it's part of its inner expression. Indentation should reflect the structure of an expression.
  • map + flatten -> flat_map.
  • |pair, something| and then pair[0]. You can de-structure arrays using the syntax |(k, v), something|.

On a first refactor, I'd write:

@nation_biopsy = User
  .where(country: @nation)
  .group_by { |u| [u.institute_type, u.biopsy] }
  .flat_map do |(institute_type, biopsy), users| 
    ["name" => institute_type, "y" => User.where(id: users.map(&:id)).sum("biopsy")]

On a second refactor, I'd try to make it work with SQL. Something like this:

@nation_biopsy = User
  .where(country: @nation)
  .group(:institute_type, :biopsy)
  .select(:institute_type, "SUM(biopsy) AS biopsy_count")
  .flat_map { |group| ["name" => group.institute_type, "y" => group.biopsy_count] }

Now, if you want to write something fancy use Arel, it allows you to write it without SQL fragments, which looks kind of cool:

users = User.arel_table
@nation_biopsy = User
  .group(users[:institute_type], users[:biopsy])
  .select(users[:institute_type], users[:biopsy].sum.as("biopsy_count"))
  .flat_map { ["name" => group.institute_type, "y" => group.biopsy_count] }
  • \$\begingroup\$ Unfortunately this gives [{"name":"Non-university hospital","y":3},{"name":"Non-university hospital","y":5},{"name":"Non-university hospital","y":16},{"name":"Non-university hospital","y":13},{"name":"University hospital","y":1}] .............in other words its not grouping on institution type \$\endgroup\$ – GhostRider Dec 13 '15 at 18:07
  • \$\begingroup\$ I don't see how "name" and "y" can end up being merged in a single hash with my code. \$\endgroup\$ – tokland Dec 13 '15 at 18:09
  • \$\begingroup\$ I just cracked it for what I needed using your code as the start, middle and almost the end. I learned a lot from reading around your code. Many thanks. \$\endgroup\$ – GhostRider Dec 13 '15 at 18:21
  • \$\begingroup\$ There are two missing brackets in the flat_maps. \$\endgroup\$ – Daniel Dec 15 '15 at 14:18
  • \$\begingroup\$ @Daniel, [:a => 1, :b => 2]is valid Ruby, inside an array they are optional. \$\endgroup\$ – tokland Dec 15 '15 at 18:28

My variant of refactor is similar:

@nation_biopsy =  
  User.where(country: @nation)
      .group(:institute_type, :biopsy)
      .map{ |(inst, biopsy), users| ["name" => inst, "y" => users.sum(&:biopsy)]}
  • \$\begingroup\$ Does this work? the output of QueryMethods#group is completely different than a Enumerable#group_by, I am not sure you can do that. \$\endgroup\$ – tokland Dec 13 '15 at 19:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.