5
\$\begingroup\$

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))
      .sum("biopsy")]}.flatten.to_json

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?

\$\endgroup\$
5
  • 1
    \$\begingroup\$ I'm more concerned that all this information is in a single User table. Tell us more about your database schema? \$\endgroup\$ Commented Dec 12, 2015 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
    Commented Dec 12, 2015 at 17:23
  • \$\begingroup\$ This snippet belongs to a model or a controller? \$\endgroup\$
    – tokland
    Commented Dec 12, 2015 at 21:04
  • \$\begingroup\$ This is really interesting, look forward to seeing a rewrite. \$\endgroup\$
    – 13aal
    Commented Dec 13, 2015 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
    Commented Dec 13, 2015 at 18:27

2 Answers 2

4
\$\begingroup\$

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")]
  end.to_json

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] }
  .to_json  

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
  .where(users[:country].eq(@nation))
  .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] }
  .to_json
\$\endgroup\$
7
  • \$\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
    Commented Dec 13, 2015 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
    Commented Dec 13, 2015 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
    Commented Dec 13, 2015 at 18:21
  • \$\begingroup\$ There are two missing brackets in the flat_maps. \$\endgroup\$
    – Daniel
    Commented Dec 15, 2015 at 14:18
  • \$\begingroup\$ @Daniel, [:a => 1, :b => 2]is valid Ruby, inside an array they are optional. \$\endgroup\$
    – tokland
    Commented Dec 15, 2015 at 18:28
0
\$\begingroup\$

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)]}
      .flatten
      .to_json
\$\endgroup\$
1
  • \$\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
    Commented Dec 13, 2015 at 19:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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