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

ElasticSearch, Tire - refactor autocomplete method for multiple resources

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

Problem

I am using ElasticSearch and Tire for search in my app across all models.

  • I would like to refactor autocomplete method because it looks to complex for me.



  • Am I using a good pattern for searching across model?



Model

class Link  'string', :index => :not_analyzed
    indexes :title, analyzer: 'snowball', boost: 100
  end

  def to_indexed_json
    to_json(
      only: [:id, :title]
    )
  end
  ...
end


Controller

class SearchController < ApplicationController
  def new
    search = SearchesCatalog.new(params[:term])

    render json: search.autocomplete.to_json
  end
end


Lib

class SearchesCatalog
  attr_reader :options

  def initialize(options = {})
    @options = HashWithIndifferentAccess.new(options)
    @resources = [ :questions, :answers, :links, :events, past_events, :reviews ]
  end

  def results
    term = options[:term]

    Tire.search @resources do
      query { string "#{term}" }
    end.results
  end

  def autocomplete
    array = []
    results.each do |result|
      case result.type
    when "past_event"
      array PastEvent", value: "/past_events/#{result.id}" }
    when "event"
      array Event", value: "/events/#{result.id}" }
    when "topic"
      array Topic", value: "/#{result.app_id}/t/#{result.id}" }
    when "link"
      array Link", value: "/links/#{result.id}" }
    when "question"
      array Question" }
    when "answer"
    array Answer" }
    end
    array
  end
end


Javascript

// set up the search bar
$("#question_search").autocomplete({
  source:$('#question_search').data('source'),
  html: true,
  minlength: 1,
  appendto: "#search_results",
  select: function( event, ui ) {
    window.location=ui.item.value;
    return false;
  },
  focus: function( event, ui ) {
    $("#question_search").val(ui.item.title);
    return false;
  },
  open: function( event, ui ) {
    $('#search_results')
    .find('a:first').addclass('first').end()
    .find('a:last').addclass('last');
  }
});

Solution

I'll take autocomplete and leave the rest for others. Some notes:

  • Never write the pattern "empty array" + each + push + return array. That's a map (more on functional programming here)



  • Is this "lib" thing really in lib/? it should go to app/. In lib you only have generic code that may be reused across applications.



  • Don't write routes by hand, ever, that's terrible practice, use the helper router methods provided by Rails.



  • Don't write tags by hand, use content_tag.



  • Abstract and simplify by identifying repeated patterns in your code.



I'd write:

def autocomplete
  results.map do |result|
    title, caption, route = case result.type.to_sym
    when :past_event
      [result.content, "PastEvent", past_event_path(result)]
    ...
    end

    label = "%s %s" % [title, content_tag(:span, caption, class: 'search-type')]
    {title: title, label: label, value: route}
  end
end

Code Snippets

def autocomplete
  results.map do |result|
    title, caption, route = case result.type.to_sym
    when :past_event
      [result.content, "PastEvent", past_event_path(result)]
    ...
    end

    label = "%s %s" % [title, content_tag(:span, caption, class: 'search-type')]
    {title: title, label: label, value: route}
  end
end

Context

StackExchange Code Review Q#19702, answer score: 2

Revisions (0)

No revisions yet.