Проблема заключается в следующем:
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 }
}
Я считаю, что намного легче читать, вместо "зацикливаться на этом, но пропустить одну итерацию, если это произойдет, а также нене делайте этого, если это произойдет ".
На самом деле это обычный шаблон, который почти каждый Rubyist уже написал дюжину раз, так что у меня фактически был фрагмент кода, который мне нужно было только немного адаптировать.Тем не менее, я покажу вам, как я мог бы поэтапно реорганизовать ваш исходный код, если у меня еще не было решения.
Сначала давайте начнем со стиля кодирования,Я знаю, что это скучно, но это важно . То, что является фактическим стилем кодирования, не важно, важно то, что код соответствует , что означает, что один фрагмент кода должен выглядеть так же, как любой другой фрагмент кода,В этом конкретном случае вы просите сообщество 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).Теперь код по крайней мере работает .
В качестве следующего шага я бы переключил назначение двух локальных переменных: сначала вы назначаете entities, затем вы назначаете summa,затем вы что-то делаете с entities и вам нужно посмотреть на три строки, чтобы выяснить, что было entities.Если вы переключите эти два параметра, использование и назначение entities будут рядом друг с другом.
В результате мы увидим, что entities назначено, затем сразу используется, а затем никогда больше не используется.Я не думаю, что это значительно улучшает читабельность, поэтому мы можем полностью от нее избавиться:
for face in Sketchup.active_model.entities
Далее следует цикл for.Это очень не-идиоматические в Ruby;Рубиисты сильно предпочитают внутренние итераторы.Итак, давайте переключимся на одно:
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face
if face.material
summa[face.material.display_name] += face.area
end
}
Одно из преимуществ, которое он имеет, заключается в том, что теперь face является локальным по отношению к телу цикла, тогда как раньше он просачивался в окружающую область.(В Ruby только собственные тела модулей, тела классов, тела методов, тела блоков и тела сценариев имеют собственную область видимости; тела циклов for и while, а также выражения if / unless / case выражения don 'т.)
Давайте перейдем к телу цикла.
Первая строка - это предложение охраны.Это хорошо, мне нравятся пункты охраны: -)
Вторая строка, ну, если face.material истина, она делает что-то, иначе ничего не делает, что означает, что цикл закончен. Итак, это еще одно охранное предложение! Тем не менее, он написан в полностью стиле, отличном от первого предложения охраны, прямо на одну строку над ним! Опять же, важна последовательность:
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face
next unless face.material
summa[face.material.display_name] += face.area
}
Теперь у нас есть два охранных пункта рядом друг с другом. Упростим логику:
Sketchup.active_model.entities.each {|face|
next unless face.kind_of? Sketchup::Face && face.material
summa[face.material.display_name] += face.area
}
Но теперь есть только один пункт охраны, охраняющий только одно выражение. Итак, мы можем просто сделать само выражение условным:
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 и & hellip; практически любой современный язык сразу же распознает этот паттерн как катаморфизм, reduce, fold, inject:into:, inject или как бы там ни назывался ваш предпочитаемый язык.
Что делает reduce, так это то, что он "сводит" несколько вещей в одну вещь. Наиболее очевидным примером является сумма списка чисел: она сводит несколько чисел к одному числу:
[4, 8, 15, 16, 23, 42].reduce(0) {|accumulator, number| accumulator += number }
[Примечание: в идиоматическом Ruby это будет написано так же, как [4, 8, 15, 16, 23, 42].reduce(:+).]
Один из способов обнаружить 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-комбинатор):
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 }
}
И это все!