HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Searching for something at a location

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
locationforsearchingsomething

Problem

I'm using Searchkick in a situation where a user might write the location they are searching for, or the thing they are searching for, or both.

I've been working on the below code for most of the day. As you can tell, I'm quite the beginner.

Ideally, what I want is something like this:

@portfolios = Portfolio.where(:published => true)
@portfolios = @portfolios.search
    params[:search] unless params[:search].nil?, 
    where: {location: {near: coords, within: "300mi"}} unless params[:where].nil?


Is there any way to do that?

I'm asking because, currently this search uses two options. It's about to become quite a bit more complicated. The next step for instance, is to let the user select the range to search in.

Here's my code:

class SearchController  true).includes([:user, :portrait]).order('created_at DESC')
            return
        end

        if params[:where].empty?
            # User entered a query but not a location
            @portfolios = Portfolio.search params[:search], include: [:user, :portrait]

        elsif params[:search].empty?
            # User entered a location but not a query
            coords = Geocoder.coordinates(params[:where])
            @portfolios = Portfolio.search where: {location: {near: coords, within: "300mi"}}, include: [:user, :portrait]

        else
            # User entered a location and a query
            coords = Geocoder.coordinates(params[:where])
            @portfolios = Portfolio.search params[:search], where: {location: {near: coords, within: "300mi"}}, include: [:user, :portrait]
        end
  end
end

Solution

Just a quick review with changes I'd make if I were working with this controller in a rails project. I'll not changes I make in a list and paste your code with the alterations I've made at the end.

  • In the third line, you have if params[:search].nil? or params[:search].empty? && params[:where].empty?. I'm assuming you're aware of the operator precedence of or vs ||, but I personally prefer to avoid the or and and keywords as they can cause confusion (both to people new to ruby and myself when I inevitably derp something up)



  • I saw a lot of duplication in checking for the presence of params[:search] and params[:where]. Repetition of common logic is usually a good candidate for a new method. I created two new methods (search_params and where_params) which preformed these checks internally and returned nil whenever the respective parameter failed the presence checks you were already performing



  • A lot of what this controller was doing was figuring out what parameters were available in order to construct a list of arguments for Portfolio.search. I'm repeating what I did with the search_params and where_params methods I created and put together a new method for constructing the desired arguments for Portfolio.search



  • I considered making another method to DRY up the code in search_params and where_params, but I figured there wasn't enough duplication here to justify another method.



Results

class SearchController < ApplicationController
  def index
    if search_params.blank? && where_params.blank?
      @portfolios = Portfolio
        .includes([:user, :portrait])
        .where(published: true)
        .order('created_at DESC')

    else
      @portfolios = search_portfolio search_params, where_params
    end
  end

  private

  # Construct a search query using search arguments
  def search_portfolio(search: nil, coordinates: nil)
    search_args = [ { include: [:user, :portrait] } ]

    # Find portfolios near a given location if coordinates
    # have been provided
    if coordinates
      search_args.unshift({
        where: {
          location: {
            near: Geocoder.coordinates(coordinates),
            within: '300mi'
          }
        }
      })
    end

    # Filter results with a search query if a search
    # term was provided
    search_args.unshift(search) if search

    Portfolio.search(*search_args)
  end

  # Return `params[:search]` if it is present and non-empty,
  # otherwise return nil
  def search_params
    params.fetch(:search, nil).present? ? params[:search] : nil
  end

  # Return `params[:where]` if it is present and non-empty,
  # otherwise return nil
  def where_params
    params.fetch(:where, nil).present? ? params[:where] : nil
  end
end

Code Snippets

class SearchController < ApplicationController
  def index
    if search_params.blank? && where_params.blank?
      @portfolios = Portfolio
        .includes([:user, :portrait])
        .where(published: true)
        .order('created_at DESC')

    else
      @portfolios = search_portfolio search_params, where_params
    end
  end

  private

  # Construct a search query using search arguments
  def search_portfolio(search: nil, coordinates: nil)
    search_args = [ { include: [:user, :portrait] } ]

    # Find portfolios near a given location if coordinates
    # have been provided
    if coordinates
      search_args.unshift({
        where: {
          location: {
            near: Geocoder.coordinates(coordinates),
            within: '300mi'
          }
        }
      })
    end

    # Filter results with a search query if a search
    # term was provided
    search_args.unshift(search) if search

    Portfolio.search(*search_args)
  end

  # Return `params[:search]` if it is present and non-empty,
  # otherwise return nil
  def search_params
    params.fetch(:search, nil).present? ? params[:search] : nil
  end

  # Return `params[:where]` if it is present and non-empty,
  # otherwise return nil
  def where_params
    params.fetch(:where, nil).present? ? params[:where] : nil
  end
end

Context

StackExchange Code Review Q#63961, answer score: 2

Revisions (0)

No revisions yet.