Search code examples
rubyhashareaassociativesketchup

Summarize object area with a Hash in Ruby

require 'sketchup'

entities = Sketchup.active_model.entities
summa = Hash.new

for face in entities
  next unless face.kind_of? Sketchup::Face
  if (face.material)
    summa[face.material.display_name] += face.area
  end
end

I'm trying to get the structure in the array as such:

summa { "Bricks" => 500, "Planks" => 4000 }

By the way, I'm making a ruby script for Google Sketchup

But if I run this code I only get 

Error: #<NoMethodError: undefined method `+' for nil:NilClass>
C:\Program Files (x86)\Google\Google SketchUp 7\Plugins\test.rb:17
C:\Program Files (x86)\Google\Google SketchUp 7\Plugins\test.rb:14:in `each'
C:\Program Files (x86)\Google\Google SketchUp 7\Plugins\test.rb:14
C:\Program Files (x86)\Google\Google SketchUp 7\Plugins\test.rb:8:in `call'

As I'm used to using PHP and just doing $array['myownassoc'] += bignumber; But I guess this isn't the right approach when using Ruby?

So any help in how I need to go would be nice.

Solution

  • The problem is this:

    summa[face.material.display_name] += face.area

    This is (roughly) equivalent to

    summa[face.material.display_name] = summa[face.material.display_name] + face.area

    However, you start out with summa as an empty hash:

    summa = Hash.new

    Which means that whenever you encounter a specific material for the first time (and obviously, this is going to be already the case in the very first iteration of the loop), summa[face.material.display_name] simply doesn't exist. So, you are trying to add a number to something that doesn't exist, which obviously cannot work.

    The quick fix would be to just initialize the hash with a default value, so that it returns something useful instead of nil for a non-existing key:

    summa = Hash.new(0)

    There are, however, a lot of other improvements that could be made to the code. Here's how I would do it:

    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 }
}

    I find that much easier to read, instead of "loop over this, but skip one iteration if that thing happens, and also don't do this if that happens".

    This is actually a common pattern, that pretty much every Rubyist has already written a dozen times, so I actually had a code snippet lying around that I only needed to slightly adapt. However, I am going to show you how I could have refactored your original code step-by-step if I hadn't already had the solution.

    First, let's start with coding style. I know it's boring, but it is important. What the actual coding style is, is not important, the important thing is that the code is consistent, which means that one piece of code should look the same as any other piece of code. In this particular instance, you are asking the Ruby community to provide you with unpaid support, so it is polite to at least format the code in a style that members of that community are used to. This means standard Ruby coding style: 2 spaces for indentation, snake_case for method and variable names, CamelCase for constants which refer to modules or classes, ALL_CAPS for constants, and so on. Don't use parentheses unless they clear up the precedence.

    In your code, for example, you use sometimes 3 spaces, sometimes 4 spaces, sometimes 5 spaces and sometimes 6 spaces for indentation, and all of that in just 9 non-empty lines of code! Your coding style is not only inconsistent with the rest of the community, it isn't even consistent with its own next line!

    Let's fix that first:

    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

    Ah, much better.

    As I already mentioned, the first thing we need to do, is fix the obvious problem: replace summa = {} (which BTW would be the idiomatic way to write it) with summa = Hash.new(0). Now, the code at least works.

    As a next step, I would switch the assignment of the two local variables: first you assign entities, then you assign summa, then you do something with entities and you have to look three lines up to figure out what entities was. If you switch the two, the usage and the assignment of entities are right next to each other.

    As a result, we see that entities is assigned, then immediately used and then never used again. I don't think this improves readability much, so we can get rid of it altogether:

    for face in Sketchup.active_model.entities

    Next comes the for loop. Those are highly un-idiomatic in Ruby; Rubyists strongly prefer internal iterators. So, let's switch to one:

    Sketchup.active_model.entities.each {|face|
  next unless face.kind_of? Sketchup::Face
  if face.material
    summa[face.material.display_name] += face.area
  end
}

    One advantage this has, is that now face is local to the body of the loop, whereas before, it was leaking out into the surrounding scope. (In Ruby, only module bodies, class bodies, method bodies, block bodies and script bodies have their own scope; for and while loop bodies as well as if/unless/case expressions don't.)

    Let's get on to the body of the loop.

    The first line is a guard clause. That's good, I like guard clauses :-)

    The second line is, well, if face.material is true-ish, it does something otherwise it does nothing, which means the loop is over. So, it's another guard clause! However, it is written in a totally different style than the first guard clause, directly one line above it! Again, consistency is important:

    Sketchup.active_model.entities.each {|face|
  next unless face.kind_of? Sketchup::Face
  next unless face.material
  summa[face.material.display_name] += face.area
}

    Now we have two guard clauses right next to each other. Let's simplify the logic:

    Sketchup.active_model.entities.each {|face|
  next unless face.kind_of? Sketchup::Face && face.material
  summa[face.material.display_name] += face.area
}

    But now there is only one single guard clause guarding only one single expression. So, we can just make the whole expression itself conditional:

    Sketchup.active_model.entities.each {|face|
  summa[face.material.display_name] += face.area if
    face.kind_of? Sketchup::Face && face.material
}

    However, that's still kind of ugly: we are looping over some collection, and then inside the loop we skip over all the items we don't want to loop over. So, if we don't want to loop over them, we do we loop over them in the first place? We don't we just select the "interesting" items first and then loop over just them?

    Sketchup.active_model.entities.select {|e|
  e.kind_of? Sketchup::Face && e.material
}.each {|face|
  summa[face.material.display_name] += face.area
}

    We can do some simplification on this. If we realize that o.kind_of? C is the same as C === o, then we can use the grep filter which uses === to pattern match, instead of select:

    Sketchup.active_model.entities.grep(Sketchup::Face).select {|e| e.material
}.each { … }

    Our select filter can further be simplified by using Symbol#to_proc:

    Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).each { … }

    Now let's get back to the loop. Anybody who has some experience in a higher-order language such as Ruby, JavaScript, Python, C++ STL, C#, Visual Basic.NET, Smalltalk, Lisp, Scheme, Clojure, Haskell, Erlang, F#, Scala, … basically any modern language at all, will immediately recognize this pattern as a catamorphism, reduce, fold, inject:into:, inject or whatever your language of choice happens to call it.

    What a reduce does, is basically it "reduces" several things into just one thing. The most obvious example is the sum of a list of numbers: it reduces several numbers into just one number:

    [4, 8, 15, 16, 23, 42].reduce(0) {|accumulator, number| accumulator += number }

    [Note: in idiomatic Ruby, this would be written just as [4, 8, 15, 16, 23, 42].reduce(:+).]

    One way to spot a reduce lurking behind a loop is to look for the following pattern:

    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

    In this case, the accumulator is the summa hash.

    Sketchup.active_model.entities.grep(Sketchup::Face).select(&:material).
reduce(Hash.new(0)) {|h, face|
  h[face.material.display_name] += face.area
  h
}

    Last but not least, I don't like this explicit returning of h at the end of block. We could obviously write it on the same line:

    h[face.material.display_name] += face.area; h

    But I prefer the use of Object#tap (aka the K-combinator) instead:

    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 }
}

    And, that's it!