[ruby-core:47726] Re: [ruby-trunk - Bug #7083][Open] Why I cannot pass the test?

On 29 September 2012 16:55, mghomn (Justin Peal) <[e-mail]> wrote:
>
> Issue #7083 has been reported by mghomn (Justin Peal).
>
> ----------------------------------------
> Bug #7083: Why I cannot pass the test?
> https://bugs.ruby-lang.org/issues/7083
>
> Author: mghomn (Justin Peal)
> Status: Open
> Priority: Normal
> Assignee:
> Category:
> Target version:
> ruby -v: ruby 1.9.3p194 (2012-04-20) [i386-mingw32]
>
>
> === Program ===
> #!/usr/bin/env ruby -w
> # encoding: utf-8
>
> require 'test/unit'
>
> class Currency
>   attr_reader :currency, :decimal_digits
>
>   def initialize currency, decimal_digits
>     @currency, @decimal_digits = currency, decimal_digits
>   end
> end
>
> class Money
>   attr_reader :hashy
>
>   def initialize hashy
>     @hashy = hashy
>     clean()
>   end
>
>   def == other
>     @hashy == other.hashy
>   end
>
>   def -@
>     @hashy.each_pair do |currency, amount|
>       @hashy[currency] = -amount
>     end
>     clean()
>   end
>
>   def + other
>     @hashy.merge!(other.hashy) do |currency, old_amount, new_amount|
>       old_amount.to_f + new_amount.to_f
>     end
>     clean()
>   end
>
>   def - other
>     @hashy.merge!(other.hashy) do |currency, old_amount, new_amount|
>       old_amount.to_f - new_amount.to_f
>     end
>     clean()
>   end
>
>   def * times
>     @hashy.each_pair do |currency, amount|
>       @hashy[currency] *= times
>     end
>     clean()
>   end
>
>   def / times
>     @hashy.each_pair do |currency, amount|
>       @hashy[currency] /= times
>     end
>     clean()
>   end
>
>   def clean
>     @hashy.each_pair do |currency, amount|
>       amount_new = amount.to_f.round(currency.decimal_digits)
>       case
>       when amount_new == 0.0
>         @hashy.delete(currency)
>       when amount_new != amount
>         @hashy[currency] = amount_new
>       end
>     end
>     self
>   end
> end
>
> class MoneyTest < Test::Unit::TestCase
>   def test_add
>     usd = Currency.new(:USD, 2)
>     cny = Currency.new(:CNY, 2)
>     eur = Currency.new(:EUR, 2)
>     jpy = Currency.new(:JPY, 0)
>     gbp = Currency.new(:GBP, 2)
>
>     assert_equal(Money.new(eur=>367.85, cny=>-337.19, jpy=>42289), Money.new(eur=>867.02, cny=>-794.75, jpy=>99675) / 2.357)
>     assert_equal(Money.new(cny=>1576.56, gbp=>752.26, jpy=>64174), Money.new(cny=>861.51, gbp=>411.07, jpy=>35068) * 1.83)
>     assert_equal(Money.new(usd=>367.04, cny=>418.27, eur=>728.18), Money.new(cny=>418.27, usd=>129.66) + Money.new(usd=>237.38, eur=>728.18))
>     assert_equal(Money.new(eur=>211.32, cny=>-980.95, jpy=>31647), -Money.new(eur=>-211.32, cny=>980.95, jpy=>-31647))
>     assert_equal(Money.new(jpy=>19627, usd=>442.39, gbp=>-393.84), Money.new(jpy=>19627, usd=>908.64) - Money.new(usd=>466.25, gbp=>393.84))
>   end
> end
> === Result ===
> Run options:
>
> # Running tests:
>
> F
>
> Finished tests in 0.029999s, 33.3344 tests/s, 166.6722 assertions/s.
>
>   1) Failure:
> test_add(MoneyTest) [C:/R/tst2.rb:87]:
> <#<Money:0x1b463f8
>  @hashy=
>   {#<Currency:0x1b56148 @currency=:JPY, @decimal_digits=0>=>19627,
>    #<Currency:0x1b561f0 @currency=:USD, @decimal_digits=2>=>442.39,
>    #<Currency:0x1b560b8 @currency=:GBP, @decimal_digits=2>=>-393.84}>> expected
> but was
> <#<Money:0x1b462f0
>  @hashy=
>   {#<Currency:0x1b56148 @currency=:JPY, @decimal_digits=0>=>19627,
>    #<Currency:0x1b561f0 @currency=:USD, @decimal_digits=2>=>442.39,
>    #<Currency:0x1b560b8 @currency=:GBP, @decimal_digits=2>=>393.84}>>.
>
> 1 tests, 5 assertions, 1 failures, 0 errors, 0 skips
>

It appears from my reading of the documentation that Hash#merge /
#merge! only runs the block when resolving duplicate keys.  This is
borne out by a simply check in IRB:

irb(main):004:0> {}.merge( {:a=>3} ){|a,b,c| p [a,b,c]; b-c }
=> {:a=>3}
irb(main):005:0> {b:1}.merge( {:a=>3} ){|a,b,c| p [a,b,c]; b-c }
=> {:b=>1, :a=>3}
irb(main):006:0> {a:6, b:1}.merge( {:a=>3} ){|a,b,c| p [a,b,c]; b-c }
[:a, 6, 3]
=> {:a=>3, :b=>1}

If I were to attempt to mend this I'd start with a less graceful
solution, like replacing the #- method with something like*:

  def - other
    self + (-(Money.new other.hashy.dup))
  end

It's a bit uglier than it could otherwise be, but all the operations
are bangy (i.e. they modify the receiving object) so I can't just use:
 `self + (-other)`, and the constructor doesn't copy the inital hash.

The multiplication operation is even weirder; I'm not sure whether
{usd=>1} * {gbp=>2} should result in {usd=>1,gbp=>2} (which it
currently does) or {usd=>0,gbp=>0} (which seems to be implied by your
interpretation of the subtraction operation).  Of course this makes
division even more weirderer, since you have potential
division-by-zero issues to resolve.

* no code in this post was actually tested by me

-- 
  Matthew Kerwin, B.Sc (CompSci) (Hons)
  http://matthew.kerwin.net.au/
  ABN: 59-013-727-651

  "You'll never find a programming language that frees
  you from the burden of clarifying your ideas." - xkcd