3

私の検索方法は臭くて肥大化しており、リファクタリングの手助けが必要です。私はRubyを初めて使用しますが、Rubyを効果的に活用する方法がわかりません。これにより、次のような肥大化したメソッドが発生します。

  # discussion.rb
  def self.search(params)
    # If there is a search query, use Tire gem for fulltext search
    if params[:query].present?
      tire.search(load: true) do
        query { string params[:query] }
      end

    # Otherwise grab all discussions based on category and/or filter
    else

      # Grab all discussions and include the author
      discussions = self.includes(:author)

      # Filter by category if there is one specified
      discussions = discussions.where(category: params[:category]) if params[:category]

      # If params[:filter] is provided, user it
      if params[:filter]
        case params[:filter]
        when 'hot'
          discussions = discussions.open.order_by_hot
        when 'new'
          discussions = discussions.open.order_by_new
        when 'top'
          discussions = discussions.open.order_by_top
        else
          # If params[:filter] does not match the above three states, it's probably a status
          discussions = discussions.order_by_new.where(status: params[:filter])
        end
      else

        # If no filter is passed, just grab discussions by hot
        discussions = discussions.open.order_by_hot
      end
    end
  end

  STATUSES   = {
    question:   %w[answered],
    suggestion: %w[started completed declined],
    problem:    %w[solved]
  }

  scope :order_by_hot,  order('...') DESC, created_at DESC")
  scope :order_by_new,  order('created_at DESC')
  scope :order_by_top,  order('votes_count DESC, created_at DESC')

これは、カテゴリでフィルタリングできる（またはフィルタリングできない）ディスカッションモデルです：question、、。problemsuggestion

すべてのディスカッションまたは単一のカテゴリは、、、、、またはでさらにフィルタリングできhotます。ステータスはモデル内のハッシュであり、カテゴリに応じていくつかの値があります（ステータスフィルタは、params [：category]が存在する場合にのみ表示されます）。newvotesstatus

複雑な問題は、Tireを使用した全文検索機能です

しかし、私のコントローラーはきれいに見えます：

  def index
    @discussions = Discussion.search(params)
  end

メタプログラミングやブロックを使用して、これを少し乾かしたりリファクタリングしたりできますか？なんとかコントローラーからこれを抽出できましたが、アイデアが足りなくなりました。私はこれをさらに進めるのに十分なRubyを知りません。

4

1 に答える 1

3

手始めに、「カテゴリやフィルターに基づいてすべてのディスカッションを取得する」は、別の方法にすることができます。

params[:filter]は何度も繰り返されるので、上部から取り出します。

filter = params[:filter]

使用できます

if [:hot, :new, :top].incude? filter
  discussions = discussions.open.send "order_by_#{filter}"
...

また、if then else ifcaseelseステートメントを除外します。私は別々のメソッドに分割して早く戻ることを好みます：

def do_something
  return 'foo' if ...
  return 'bar' if ...
  'baz'
end

discussions = discussions...何度も表示されますが、奇妙に見えます。return discussions...代わりに使用できますか？

STATUSES定数が最後に表示されるのはなぜですか？通常、定数はモデルの上部に表示されます。

リファクタリングする前に、必ずすべてのテストを作成してください。

についてのコメントに返信するにはreturn 'foo' if ...：

検討：

def evaluate_something
  if a==1
    return 'foo'
  elsif b==2
    return 'bar'
  else
    return 'baz'
  end
end

これを次のようにリファクタリングすることをお勧めします。

def evaluate_something
  return 'foo' if a==1
  return 'bar' if b==2
  'baz'
end

おそらく、if..then..else..ifステートメントの一部をリファクタリングすることができます。

おすすめの本：クリーンコード

于 2012-11-01T14:29:32.987 に答える