Ask A Question

Notifications

You’re not receiving notifications from this thread.

Performance increase over group_by

arup rakshit asked in Rails

How can I write the below operation, being handled by group_by Ruby method, in terms of DB specific query?

def list_users
    @search_by_options = [:age, :location, :department, :designation]
    @users = User.all.group_by { |user| user.public_send(params[:search_by] || :location) }
  end

Corresponding view code is as below :-

<%= form_tag list_users_users_path, :method => 'get' do %>
  <p>
  <%= select_tag "search_by", options_for_select(@search_by_options) %>
  <%= submit_tag "Submit"%>
  </p>
<% end %>
<% @users.each do |grouping_key, users| %>
  <p> <%= grouping_key %> : <%= users.map(&:name).join("||")%></p>
<% end %>
<%= link_to "Back to Main page", root_path %>
Reply

Are you trying to implement a search or just sorting into groups? Depending on the goal, I have a few different suggestions.

Reply

Thanks for your reply!

No, I am just trying to display users names group by location/age/department etc. Currently nothing I think about for searching.

Reply

In that case, I think your approach is fine. A group by query isn't really going to help you much in SQL because you still need to sort it on the Ruby side. The SQL group by is most useful when you're gathering things like counts or doing joins, not for ordering results.

The Ruby group_by method is actually what you want here to organize your results. I would maybe rename @users to @grouped_users so that it is clear the variable is not an array of users but a grouped array.

Reply

Thanks again!

How can I refactor the view code, and extract out as much as Ruby code from that ? What is your suggestion regarding the searching as you mentioned in your earlier comment?

Reply

You could take advantage of Ransack for the search there. It has all kinds of nice things that can take care of searching and sorting. Take a look at the episode I did on it: https://gorails.com/episodes/forum-search-with-ransack

Two changes I would suggest, but depending on what you're trying to achieve, they might not be relevant:

  1. Your search by options aren't safe in that I can type ?search_by=destroy and it will send method call that to the user instance which could delete the records for example. That's a potential security problem so you will probably want to validate that better.
    @search_by_options = [:age, :location, :department, :designation]
    # Verify the search_by column is valid, otherwise default to location
    @search_column = @search_by_options.include?(params[:search_by]) ? params[:search_by] : :location
  1. If you don't plan on linking the user names anywhere, you could do the map inside the controller instead. That would make it a little bit more obvious in the views.
    @grouped_user_names = User.all.group_by { |user| 
      user.public_send(params[:search_by] || :location)
    }.map(&:name).join("||")
<% @grouped_user_names.each do |grouping_key, user_names| %>
  <p> <%= grouping_key %> : <%= user_names %></p>
<% end %>

And since the controller mapping is reasonably nasty, you could put that inside a class method on User or you could create a presenter class to handle the logic. I'd go with the class method at first until you have a handful that can be refactored into a presenter.

Reply

Thanks for your suggestions!

Reply
Join the discussion
Create an account Log in

Want to stay up-to-date with Ruby on Rails?

Join 87,110+ developers who get early access to new tutorials, screencasts, articles, and more.

    We care about the protection of your data. Read our Privacy Policy.