Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

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

  1. I would like to refactor autocomplete method because it looks to complex for me.
  2. Am I using a good pattern for searching across model?


class Link < ActiveRecord::Base
  include Tire::Model::Search
  include Tire::Model::Callbacks

  tire.mapping do
    indexes :id, :type => 'string', :index => :not_analyzed
    indexes :title, analyzer: 'snowball', boost: 100

  def to_indexed_json
      only: [:id, :title]


class SearchController < ApplicationController
  def new
    search =[:term])

    render json: search.autocomplete.to_json


class SearchesCatalog
  attr_reader :options

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

  def results
    term = options[:term] @resources do
      query { string "#{term}" }

  def autocomplete
    array = []
    results.each do |result|
      case result.type
    when "past_event"
      array << { title: result.content, label: "#{result.content} <span class='search-type'>PastEvent</span>", value: "/past_events/#{}" }
    when "event"
      array << { title: result.title, label: "#{result.title} <span class='search-type'>Event</span>", value: "/events/#{}" }
    when "topic"
      array << { title:, label: "#{} <span class='search-type'>Topic</span>", value: "/#{result.app_id}/t/#{}" }
    when "link"
      array << { title: result.title, label: "#{result.title} <span class='search-type'>Link</span>", value: "/links/#{}" }
    when "question"
      array << { title: result.content, label: "#{result.content[0..100]} <span class='search-type'>Question</span>" }
    when "answer"
    array << { title: result.content, label: "#{result.content[0..100]} <span class='search-type'>Answer</span>" }


// set up the search bar
  html: true,
  minlength: 1,
  appendto: "#search_results",
  select: function( event, ui ) {
    return false;
  focus: function( event, ui ) {
    return false;
  open: function( event, ui ) {
share|improve this question
up vote 2 down vote accepted

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 do |result|
    title, caption, route = case result.type.to_sym
    when :past_event
      [result.content, "PastEvent", past_event_path(result)]

    label = "%s %s" % [title, content_tag(:span, caption, class: 'search-type')]
    {title: title, label: label, value: route}
share|improve this answer
Using routes looks great but I have problem when item not exist, for example: [result.content, "Review", service_provider_path(result.service_provider)] I would like to skip adding that item to hash if result.service_provider.nil? Is possible to not adding it to hash {title: title, label: label, value: route}? – regedarek Dec 18 '12 at 14:47
write (service_provider_path(result.service_provider) if result.service_proder) – tokland Dec 18 '12 at 14:51

One suggestion is to create three virtual attributes (title, label, link) for each and every model and add the custom implementations. Add these attributes to the index mapping.

Then you have a unique interface to all models. That way you can get rid of the case statement completely. do |result|
  {title: result.title, label: result.label, value: url}

It will make your index bigger but saves computation time on the autocomplete action.

share|improve this answer
+1 That's the orthodox approach for OOP and introduces an interesting matter, the so-colled "Expression problem": Personally I've found that, when writing helpers, it's more maintainable to use cases that to scatter code throughout the models, though. In this case it should be more something like presenters, models are not really the place for "label" or "url" methods. – tokland Dec 19 '12 at 7:41

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

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