4
\$\begingroup\$

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?

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
  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 << { title: result.content, label: "#{result.content} <span class='search-type'>PastEvent</span>", value: "/past_events/#{result.id}" }
    when "event"
      array << { title: result.title, label: "#{result.title} <span class='search-type'>Event</span>", value: "/events/#{result.id}" }
    when "topic"
      array << { title: result.name, label: "#{result.name} <span class='search-type'>Topic</span>", value: "/#{result.app_id}/t/#{result.id}" }
    when "link"
      array << { title: result.title, label: "#{result.title} <span class='search-type'>Link</span>", value: "/links/#{result.id}" }
    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>" }
    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');
  }
});
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

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
\$\endgroup\$
2
  • \$\begingroup\$ 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}? \$\endgroup\$ Dec 18, 2012 at 14:47
  • \$\begingroup\$ write (service_provider_path(result.service_provider) if result.service_proder) \$\endgroup\$
    – tokland
    Dec 18, 2012 at 14:51
2
\$\begingroup\$

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.

results.map do |result|
  {title: result.title, label: result.label, value: url}
end

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

\$\endgroup\$
1
  • 1
    \$\begingroup\$ +1 That's the orthodox approach for OOP and introduces an interesting matter, the so-colled "Expression problem": c2.com/cgi/wiki?ExpressionProblem. 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. \$\endgroup\$
    – tokland
    Dec 19, 2012 at 7:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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