Adding ability to temporarily change the rounding mode

[hadees]

I added the ability to temporarily change the rounding mode in a block passed to Money.rounding_mode. I needed this feature because I'm using Stripe as my payment processor and they use a different rounding mode then I am. So this makes it easier for me to estimate their fees.

[ohthatjames]

I can see 2 issues around this:

1. It's not thread-safe: if two threads call Money.rounding_mode then you may get inconsistent results.
2. If the yield fails for any reason you may be left in an inconsistent state because you haven't set the rounding mode back. Suggest you use an ensure to set the rounding mode back.

[hadees]

Fair points, I was also thinking maybe rounding_mode should return yield results instead of the current rounding mode because when you are temporarily changing it getting back the current mode isn't really that useful but getting back the results of a calculation would be.

What would be the right way to make it thread safe?

[semmons99]

Closing, please reopen if you're actively working on this.

[hadees]

I'd like to work on it but I didn't get any other feedback about how best to make it thread safe.

[semmons99]

@ohthatjames do you have more suggestions?

[hadees]

@semmons99 I've gone ahead and made changes in my branch that I think will make it thread safe. I can't seem to reopen the pull request though.

[hadees]

Should I just create a new pull request?

[semmons99]

@hadees reopened

[hadees]

I also want to update the YARD docs before this gets merged in but I'm not sure the right syntax to show it returns [BigDecimal::ROUND_MODE] or the results of the block.

[semmons99]

I believe you're looking for @yard, @yardparam and @yardreturn http://rubydoc.info/gems/yard/file/docs/Tags.md#yield

[hadees]

That was my last update to the branch unless anyone has more suggestions.

[semmons99]

looks good

[ohthatjames]

Really sorry, I've been away and I missed the notifications on this one. Two observations:

1. this doesn't allow for nested blocks (the following example fails):

it "allows nested blocks" do
  Money.rounding_mode(BigDecimal::ROUND_UP) do
    Money.rounding_mode(BigDecimal::ROUND_DOWN) do
      Money.new(1.9).fractional
    end.should == 1

    Money.new(1.1).fractional
  end.should == 2
end

This is caused by 32929d8#diff-a6d34f32282d8d431a585cdaf0aaf730R174 - we may have to make it an array and push and pop rounding modes.

2. I've been thinking about this, and I can't really see the use-case for doing this on a global level. Would it be safer to allow the rounding mode to be overridden on each method that uses it? It adds a certain amount of complexity and finding bugs due to changing the rounding may be a bit annoying (action at a distance). Do you have any example code of how you intend to use this?

[ohthatjames]

BTW, happy to look into the nested blocks issue if others think it's worth closing off.

[semmons99]

@ohthatjames it would be nice to have the nested blocks issue fixed, but I don't think it's something that's high priority.