Pourquoi RuboCop suggère-t-il de remplacer .times.map par Array.new?

Question

RuboCop suggère:

Utiliser Array.new avec un bloc au lieu de .times.map.

Dans les docs de la conférence des parties:

Ce cop vérifie .fois.carte des appels. Dans la plupart des cas, ces appels peuvent être remplacés par un explicite de la matrice de la création.

Exemples:

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

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

Je sais qu'il peut être remplacé, mais j'ai le sentiment 9.times.map est plus proche de la grammaire anglaise, et il est plus facile de comprendre ce que fait le code.

Pourquoi devrait-il être remplacé?

Answer 1

Le dernier est plus performant, voici une explication: Pull request où ce flic a été ajouté

Il vérifie pour les appels comme ceci:

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

et suggère d'utiliser plutôt ceci:

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

Le nouveau code a environ la même taille, mais utilise moins de méthode appels, consomme moins de mémoire, travaille un petit peu plus vite, et à mon avis est plus lisible.

J'ai vu de nombreuses occurrences de fois.{carte,collecter} dans différents des projets connus: Rails, GitLab, Rubocop et plusieurs closed-source des apps.

Repères:

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

Je ne suis pas sûr maintenant que la fibre est le bon espace de noms pour la cop. Laissez - moi savoir si je dois le déplacer vers la Performance.

Je n'ai pas l'implémenter l'autocorrection, car il peut potentiellement pause le code existant, par exemple si quelqu'un a Fixnum#fois la méthode redéfinie faire quelque chose de fantaisie. L'application de l'autocorrection de casser leur code.

Answer 2

Si vous sentez que c'est plus lisible, aller avec elle.

C'est une règle de performance et la plupart des codepaths dans votre application sont probablement pas critique pour les performances. Personnellement, je suis toujours ouvert à la faveur de la lisibilité sur l'optimisation prématurée.

Cela dit

100.times.map { ... }

• times crée un Enumerator objet
• map énumère sur cet objet, sans être en mesure d'optimiser, par exemple, la taille du tableau n'est pas connue à l'avance et qu'il pourrait avoir à réaffecter plus d'espace de manière dynamique et il a de les énumérer sur les valeurs en appelant Enumerable#each depuis map est mis en œuvre que de façon

Alors que

Array.new(100) { ... }

• new alloue un tableau de taille N
• Et puis utilise un natif de la boucle pour remplir les valeurs

Answer 3

Lorsque vous avez besoin de la carte le résultat d'un bloc invoqué un montant fixe de temps, vous avez le choix entre:

Array.new(n) { ... }

et:

n.times.map { ... }

Ce dernier est d'environ 60% de plus pour n = 10, qui va jusqu'à environ 40% pour n > 1_000.

Note: échelle logarithmique!