問題はこれです:
summa[face.material.display_name] += face.area
これは(ほぼ)同等です
summa[face.material.display_name] = summa[face.material.display_name] + face.area
ただし、summa
空のハッシュとして開始します。
summa = Hash.new
つまり、特定のマテリアルに初めて遭遇したときはいつでも(そして、明らかに、これはループの最初の反復ですでに当てはまります)、summa[face.material.display_name]
単に存在しません。したがって、存在しないものに番号を追加しようとしていますが、これは明らかに機能しません。
手っ取り早い解決策は、ハッシュをデフォルト値で初期化することです。これにより、nil
存在しないキーの代わりに有用なものが返されます。
summa = Hash.new(0)
ただし、コードに加えることができる他の多くの改善があります。これが私がそれをする方法です:
require 'sketchup'
Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).
reduce(Hash.new(0)) {|h, face|
h.tap {|h| h[face.material.display_name] += face.area }
}
「これをループするのではなく、はるかに読みやすいと思いますが、そのようなことが起こった場合は1回の反復をスキップし、それが起こった場合はこれを行わないでください」。
これは実際には一般的なパターンであり、ほとんどすべてのRubyistがすでに12回書いているので、実際には、わずかに適応するだけでよいコードスニペットがありました。ただし、まだ解決策がなかった場合に、元のコードを段階的にリファクタリングする方法を紹介します。
まず、コーディングスタイルから始めましょう。退屈なことは知っていますが、それは重要です。実際のコーディングスタイルが何であるかは重要ではありません。重要なことは、コードが一貫していることです。つまり、1つのコードが他のコードと同じように見える必要があります。この特定の例では、Rubyコミュニティに無料のサポートを提供するように依頼しているので、少なくともそのコミュニティのメンバーが慣れているスタイルでコードをフォーマットするのは礼儀正しいことです。これは、標準のRubyコーディングスタイルを意味します。インデント用の2つのスペース、メソッド名と変数名用のsnake_case、モジュールまたはクラスを参照する定数用のCamelCase、定数用のALL_CAPSなどです。優先順位が明確にならない限り、括弧は使用しないでください。
たとえば、コードでは、インデントに3つのスペース、4つのスペース、5つのスペース、6つのスペースを使用し、それらすべてを9行の空でないコードで使用します。あなたのコーディングスタイルは、コミュニティの他の部分と矛盾しているだけでなく、それ自体の次の行とも矛盾しています!
最初にそれを修正しましょう:
require 'sketchup'
entities = Sketchup.active_model.entities
summa = {}
for face in entities
next unless face.kind_of? Sketchup::Face
if face.material
summa[face.material.display_name] += face.area
end
end
ああ、はるかに良い。
すでに述べたように、最初に行う必要があるのは、明らかな問題を修正することです。summa = {}
(ところで、これを書くための慣用的な方法です)を。に置き換えsumma = Hash.new(0)
ます。これで、コードは少なくとも機能します。
次のステップとして、2つのローカル変数の割り当てを切り替えます。最初に割り当てentities
、次に割り当てsumma
、次に何かを実行し、entities
3行を調べて何entities
があったかを把握する必要があります。2つを切り替えると、使用法と割り当てentities
がすぐ隣になります。
その結果、それentities
が割り当てられ、すぐに使用され、その後二度と使用されないことがわかります。これによって読みやすさが大幅に向上するとは思わないので、完全に取り除くことができます。
for face in Sketchup.active_model.entities
次はfor
ループです。これらはRubyでは非常に非慣用的です。Rubyistは、内部イテレータを強く好みます。それでは、1つに切り替えましょう:
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face
if face.material
summa[face.material.display_name] += face.area
end
}
これが持つ利点の1つはface
、以前はループの本体にローカルであったのに対し、以前は周囲のスコープに漏れていたことです。(Rubyでは、モジュール本体、クラス本体、メソッド本体、ブロック本体、およびスクリプト本体のみが独自のスコープを持ちます。ループ本体for
と//式にはありません。)while
if
unless
case
ループの本体に取り掛かりましょう。
最初の行はガード句です。それは良いです、私はガード条項が好きです:-)
2行目は、face.material
本当の場合は何かを実行し、それ以外の場合は何も実行しません。つまり、ループが終了します。だから、それは別のガード条項です!ただし、最初のガード句とはまったく異なるスタイルで書かれており、そのすぐ上に1行あります。繰り返しますが、一貫性が重要です。
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face
next unless face.material
summa[face.material.display_name] += face.area
}
これで、2つのガード句が隣り合っています。ロジックを単純化しましょう:
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face && face.material
summa[face.material.display_name] += face.area
}
しかし、現在は、単一の式のみを保護する単一のガード句が1つだけあります。したがって、式全体を条件付きにすることができます。
Sketchup.active_model.entities.each {|face|
summa[face.material.display_name] += face.area if
face.kind_of? Sketchup::Face && face.material
}
ただし、それでも少し醜いです。コレクションをループしてから、ループ内でループしたくないすべてのアイテムをスキップします。それで、それらをループしたくない場合は、そもそもそれらをループしますか?最初に「興味深い」アイテムを選択してから、それらだけをループするだけではありませんか?
Sketchup.active_model.entities.select {|e|
e.kind_of? Sketchup::Face && e.material
}.each {|face|
summa[face.material.display_name] += face.area
}
これを簡単にすることができます。o.kind_of? C
これがと同じであることがわかった場合は、次の代わりに、パターンマッチングに使用するフィルターを使用C === o
できます。grep
===
select
Sketchup.active_model.entities.grep(Sketchup::Face).select {|e| e.material
}.each { … }
select
フィルタは、次を使用してさらに簡略化できますSymbol#to_proc
。
Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).each { … }
それでは、ループに戻りましょう。Ruby、JavaScript、Python、C ++ STL、C#、Visual Basic.NET、Smalltalk、Lisp、Scheme、Clojure、Haskell、Erlang、F#、Scalaなどの高次言語の経験がある人…基本的にはあらゆる現代言語とにかく、このパターンはreduce
、カタモルフィズム、、、、、または選択した言語で呼び出されるものとしてすぐに認識されます。fold
inject:into:
inject
基本的には、いくつかのものreduce
を1つに「削減」します。最も明白な例は、数のリストの合計です。これは、いくつかの数を1つの数に減らします。
[4, 8, 15, 16, 23, 42].reduce(0) {|accumulator, number| accumulator += number }
[注:慣用的なRubyでは、これは同じように記述され[4, 8, 15, 16, 23, 42].reduce(:+)
ます。]
ループの背後に潜んでいるものを見つける1つの方法reduce
は、次のパターンを探すことです。
accumulator = something # create an accumulator before the loop
collection.each {|element|
# do something with the accumulator
}
# now, accumulator contains the result of what we were looking for
この場合、accumulator
はsumma
ハッシュです。
Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).
reduce(Hash.new(0)) {|h, face|
h[face.material.display_name] += face.area
h
}
h
大事なことを言い忘れましたが、私はこのブロックの終わりに明示的に戻るのが好きではありません。明らかに同じ行に書くことができます:
h[face.material.display_name] += face.area; h
しかし、私はObject#tap
代わりに(別名K-combinator)の使用を好みます:
Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).
reduce(Hash.new(0)) {|h, face|
h.tap {|h| h[face.material.display_name] += face.area }
}
以上です!