Summing Moneys

[olivier-thatch]

We ran into an interesting bug today. We had code that looked like this:

foos.sum(&:amount) == expected_total

where foos is a collection that has an amount property that returns a Money, and expected_total is also a Money.

This works fine, except when foos is empty. In that case, the default value 0 (an Integer) is used, and as it turns out:

0 == Money.new(0, "USD") # => false

The fix is easy enough, just provide an explicit initial value:

foos.sum(Money.new(0, "USD"), &:amount) == expected_total

but this still seems like a very easy way for developers to shoot themselves in the foot when dealing with Money. I was wondering if Shopify (or others) have dealt with this issue before and have any recommendations for avoiding this issue.

One thought I had was a Rubocop rule that requires an explicit initial value for all #sum calls, but that seems overkill and annoying since it works fine when dealing with non-Money numerics.

[elfassy]

Interesting. I don't think we should have 0 == Money.new(0, "USD") # => true. This could lead to other wrong assumptions. The only thing I can think of (other than the rubocop rule you mentioned) is creating a helper to do sums

# Before
foos.sum(Money.new(0, "USD"), &:amount)

# After
Money.sum(foos, &:amount)

Devs would still know to use it instead of the regular sum, so would likely also require a rubocop rule to enforce. Open to other suggestions

[olivier-thatch]

Unfortunately I don't think this could be enforced via Rubocop, since Rubocop cannot statically know that foos.sum(&:amount) returns a Money.

There's probably no great solution here... Maybe Money#==(other) could emit a warning when other is a Numeric? This would keep the current behavior but make the issue a bit more obvious.