2017-01-07 3 views
13

RuboCopが示唆:警官のためのdocsに代わり.times.map.RuboCopが.times.mapをArray.newに置き換えることを提案するのはなぜですか?

のブロックと

使用Array.new:.times.mapコール用

この警官のチェックを。ほとんどの場合、そのような呼び出しは、明示的な配列作成で置き換えることができます。

例:

# bad 
9.times.map do |i| 
    i.to_s 
end 

# good 
Array.new(9) do |i| 
    i.to_s 
end 

私はそれを置き換えることができます知っているが、私は9.times.mapは、英語の文法に近い感じ、そしてそれは、コードが何をするかを理解する方が簡単です。

なぜ交換する必要がありますか?

+3

注意。 :-) – ruakh

+0

すべてのパフォーマンスの警官は、https://github.com/JuanitoFatas/fast-rubyから適応されました:-) – Drenmi

答えて

11

最後はよりパフォーマンスで、ここでの説明です:

9.times.map { |i| f(i) } 
9.times.collect(&foo) 

、代わりにこれを使用することを提案:

Array.new(9) { |i| f(i) } 
Array.new(9, &foo) 

Pull request where this cop was added

それは、このような呼び出しをチェック新しいコードはおおよそ同じサイズですが、より少ないメモリしか使用しないで、 呼び出しを使用します。私の意見では少し速くorksして が読みやすくなります。

Rails、GitLab、Rubocop、およびいくつかのクローズドソース アプリで、別の の別のプロジェクトで{map、collect}を何度も見てきました。

ベンチマーク:

Benchmark.ips do |x| 
    x.report('times.map') { 5.times.map{} } 
    x.report('Array.new') { Array.new(5){} } 
    x.compare! 
end 
__END__ 
Calculating ------------------------------------- 
      times.map 21.188k i/100ms 
      Array.new 30.449k i/100ms 
------------------------------------------------- 
      times.map 311.613k (± 3.5%) i/s -  1.568M 
      Array.new 590.374k (± 1.2%) i/s -  2.954M 

Comparison: 
      Array.new: 590373.6 i/s 
      times.map: 311612.8 i/s - 1.89x slower 

私はリントが警官の正しい名前空間であることを、今はよく分かりません。パフォーマンスに移動する必要があるかどうかを私に知らせてください。

また、私は自動修正を実装していませんでした。潜在的に という既存のコードを破る可能性があるためです。もし誰かがFixnum#timesメソッドを持っていれば、何かを気に入るように を再定義します。 autocorrectionを適用すると、コードが壊れてしまいます。

+1

ええ、それは実際には良い考えのようです。正しいサイズの配列を事前に割り当てます。 'times'は' map'まで頭を上げませんので、推測して潜在的にサイズを変更する必要があります。 – tadman

+1

リンクを投稿するだけではなく、リンクされたコンテンツを段落に要約してください。 – akuhn

+0

リンクではなぜそれが遅いのか説明していません。 – akuhn

7

あなたはそれがより読みやすいと感じる場合。

これはパフォーマンスルールであり、アプリケーションのほとんどのコードパスはパフォーマンスに重大な影響を与えません。個人的には、早すぎる最適化よりも読みやすさを優先するように私はいつも開いています。

100.times.map { ... } 
  • times
  • mapは、例えば、配列のサイズは先行知られておらず、それがする必要がある場合があり、最適化できず、そのオブジェクト上に列挙Enumeratorオブジェクトを作成し、前記

    より多くのスペースを動的に再割り当てし、Enumerable#eachを呼び出すことによって値を列挙する必要があります。mapがそのように実装されているからです。

Array.new(100) { ... } 
  • newはサイズN
  • の配列を割り当てそして、あなたはブロックの結果をマップする必要がある場合の値
2

を埋めるためにネイティブのループを使用してのに対し一定の時間が呼び出された場合、次の間のオプションがあります。

Array.new(n) { ... } 

と:

n.times.map { ... } 

後者はn > 1_000ために約40%までダウンn = 10、約60%遅くなります。

注:対数スケール!ヒントのビットは、(あなたがあなたのリンクから見ることができるように)、それは「パフォーマンス」グループ内だと

enter image description here

関連する問題