3
\$\begingroup\$

I am using the ruby sunspot gem to create a search users function on my site. It works fine and the users controller for the index page where there is a search form is this

def index
 if params[:search].nil?
  @users = nil
 else
  @search = User.search do
    fulltext params[:search]
  end
  @users = @search.results.paginate(page: params[:page], :per_page => 10)
 end
end

on the index page, I do this

  <% if @users %>
    <ul class="users followers">
      <%= render @users %>
      <%= will_paginate @users%>
    </ul>
  <% end %>

My question is this. I think setting @users to nil looks and feels wrong. I want the index page to show nothing but my search form if nothing is put in the field. There must be some easy Ruby way to express this. By the way I am fairly new to Ruby on Rails.

\$\endgroup\$
  • \$\begingroup\$ I like @dankohn's suggestion for tightening up the controller code. I don't have a problem with the if @users in your view code, but I do think you should change the name from users to something more descriptive, perhaps user_search_results? \$\endgroup\$ – Jonah Apr 11 '15 at 23:51
3
\$\begingroup\$

How about using the ternary operator and assigning the result to the variable?

def index
  @users = params[:search].nil? ? nil : User
           .search { fulltext params[:search] }.results
           .paginate(page: params[:page], per_page: 10)
end
\$\endgroup\$
  • \$\begingroup\$ Works nicely. still unsure about setting @users equal to nil though \$\endgroup\$ – GhostRider Apr 12 '15 at 7:25
  • \$\begingroup\$ Maybe @users = if params[:search] would be more clear than a ternary where a branch returns nil? \$\endgroup\$ – tokland Apr 12 '15 at 9:12
0
\$\begingroup\$

You should consider returning always the same type of results for all cases in order to keep things consistent. Right now in case there are no search parameters you return an nil object for users, while in the case there are parameters you return a collection of user results. What if the search doesn't match anything? You still have no users but in this case you get an empty collection.

You could have something like this in your controller action:

def index
  @users =
    if params[:search].present?
      @search = User.search { fulltext(params[:search]) }
      @search.results.paginate(page: params[:page], per_page: 10)
    else
      []
    end
end

and something like this in your view:

<% if @users.present? %>
  <ul class="users followers">
    <%= render @users %>
    <%= will_paginate @users %>
  </ul>
<% end %>

This way you know that @users will always be a collection even if it has no items and your code will be consistent.

For more information/opinions on these cases take a look to this article.

Note: Keep in mind that [].present? and {}.present? are methods introduced by Ruby on Rails, not Ruby. You could have achieved the same thing with @users && !@users.empty?.

\$\endgroup\$

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.