checkout.rb

RULES = { "A" => 50, 
          "B" => 30,
          "C" => 20,
          "D" => 15
        }

DISCOUNTS = { "A" => [3, 20],
              "B" => [2, 15]
            }

class CheckOut

  def initialize(rules)
    @counts = Hash.new(0)
  end

  def scan(item)
    @counts[item] += 1
  end

  def sum
    if @counts == {}
      0
    else
      @counts.map { |item, count| RULES[item] * count }.inject(:+)
    end
  end

  def total
    sum - discount
  end

  def discount
    DISCOUNTS.map { |item, (required, discount)| 
      (@counts[item] / required) * discount }.inject(:+)
  end

end

This looks really good! I like that you've learned how to use inject to sum things up quickly. The map+inject technique you've stumbled onto here is often called map-reduce, and it's actually at the core of a lot of functional programming, so good job. I also really like how short you've managed to make most of these functions. This is so much cleaner than your first version of this -- I'm really impressed!

Here are some ways you could push your solution even further:

Right now, in CheckOut#total, you're using an unless-else construction. Since these require people reading your code to think in double negatives, it's considered good practice to invert them and make them if-else statements instead. I'm guessing that you did that here because you wanted to have the 0 last, since it's the less likely case? That was a good instinct, but in practice, people will see the unless-else and get annoyed enough that they won't see the point you were trying to make with it. 😄

But let's dig a little deeper. You threw that in because you were trying to overcome that frustrating error where sum was sometimes nil, yeah? And when you try to subtract from nil it throws a NoMethodError. The formal term for what you did is "nil-checking."

In a lot of other languages, the compiler would have yelled at you in a different place - not when you tried to subtract an amount from nil, but instead when you tried to return nil as sum. In what are called "strongly typed" languages (think Java), methods are only allowed to return one type of object - if a method returns an Integer in most cases, it must return an Integer in all cases, no exceptions. So, the compiler in a strongly typed language would have thrown an error on L21. It would have done this because of the error you were trying to solve by nil-checking in total -- if a method can return objects that have different "interfaces" (that implement different sets of methods) then you set yourself up for an error later when a method that uses that object expects it to have a method that it doesn't have.

Nil-checking is often considered a "code smell" because if you need to nil-check, it means that elsewhere in your code you set up a method that had an inconsistent return type. Generally, the fix for nil-checks is to fix the root cause -- the method that has the inconsistent return type, namely sum.

Luckily, that's really easy here! You can just return 0 as the sum if nothing has been checked out yet.

I have some other feedback, after you've digested what I just said, but I figure that's plenty to start with. :) And I do want to stress that I am super impressed by how much cleaner this version is than the version you showed me last Saturday. It's a huge improvement.