複数のアレイ/ハッシュルックアップを変数

Question

に格納されなければならない、私は私のコードや臭いの1をリファクタリングする最近Reekを使用してきた、DuplicateMethodCall、このようarray[1]やhash[:key]として、配列やハッシュのルックアップに呼び出されています複数回と呼ばれる。

私は複数の配列やハッシュ検索が非常に高価なので、変数を直接呼び出すのではなく変数に格納する必要があるのだろうかと思っていました。

変数内で複数のオブジェクトメソッド呼び出し（特にDB呼び出しの場合）を格納することをためらうことはありませんが、配列とハッシュの検索では、これは過度の気分になります。

例えば、私は、コードのこの部分のための警告を取得します：

def sort_params 
    return [] if params[:reference_letter_section].nil? 

    params[:reference_letter_section].map.with_index(1) do |id, index| 
     { id: id, position: index } 
    end 
    end 

が、私は、独自の変数にparams[:reference_letter_section]を保存するように感じるが

Answer 1

は、だから私はあればと思いまし多すぎます複数の配列またはハッシュ検索は非常に高価です

高額なコールだけが複数回コールをしない理由ではありません。また、実際の必要性なしにコードを混乱させる。このそれほど不自然ではない例を考えてみましょう：

Order.new(
    name:  params[:order][:name], 
    total:  params[:order][:total], 
    line_items: [ 
    { 
     product_name: params[:order][:line_item][:product], 
     price:  params[:order][:line_item][:price], 
    } 
    ] 
) 

これらのハッシュアクセスが超安いにもかかわらず、それはまだ読みやすさの理由のために、それらを抽出するために理にかなっています。

order_params  = params[:order] 
line_item_params = order_params[:line_item] 

Order.new(
    name:  order_params[:name], 
    total:  order_params[:total], 
    line_items: [ 
    { 
     product_name: line_item_params[:product], 
     price:  line_item_params[:price], 
    } 
    ] 
) 

Answer 2

重複したハッシュ検索は、これらの2行のコード間の結合を表します。これにより、コードを理解するのにかかる時間が長くなり、コードを変更するときに摩擦の原因となる可能性があります。もちろん、このような小さな方法の中では、コストは比較的低いです。 2つのコード行が離れている場合（例えば、異なるクラスで）、カップリングの効果ははるかに高価になります。

def sort_params 
    reference_letters = params[:reference_letter_section] || [] 
    reference_letters.map.with_index(1) do |id, index| 
    { id: id, position: index } 
    end 
end