Fix precision representation of floats

[javierhonduco]

Background

When performing string concatenations of Measured::Weight.new(<number>, :lbs) with a number that cannot be represented as a float, it is shown with too many decimals:

Measured::Weight.new(9.3, :lbs).to_s
=> "9.300000000000001 lb"

Research

@benwah and I had a look of the history of the changes when we saw that the rounding was being set to Float::DIG + 1.

This behaviour was introduced in this measured PR, mimicking Ruby's float#to_d implementation behaviour.

However, with the current implementation of Float::DIG + 1, this is what we get:

[2] pry(main)> BigDecimal(9.3, Float::DIG + 1).to_s
=> "9.300000000000001"

and with Float::DIG:

[3] pry(main)> BigDecimal(9.3, Float::DIG + 1).to_s
=> "9.3"

while float#to_d:

[4] pry(main)> 9.3.to_d.to_s
=> "9.3"

This was, indeed, changed in Ruby, as it can be seen in this Ruby PR. If you are curious, it was introduced here, this bug only lasted 5 days or so in trunk 😄 .

Proposed implementation

Changing BigDecimal(value, Float::DIG + 1) to value.to_d, to use Ruby's implementation instead of calling with our defined precision 😃 .

This, however, leaves the code style a bit inconsistent, as the other expressions of the when use the constructor instead of a method call on themselves, but thought I would change that in another PR, if that sounds good :)

Given that this change is Ruby version dependant, I think we should maybe bump the version a bit more than I was expecting at first (minor or more?), this only happen in older Rubies, though.

What do you think? 😄

[javierhonduco]

Thanks!! 😄

Related to the version, shall we bump the patch or the minor version? Just pushed a version bumping the patch field, but let me know what you think

[thegedge]

In theory, I'd consider deferring this to a major change, because it's actually a backwards-incompatible behavioural change. That being said, the old behaviour was wrong, so I'm okay bumping minor (hopefully we won't be breaking anyone!)

[javierhonduco]

Cool, thanks! 😄

Will merge and push to Rubygems as soon as I verify that our dependent project do not break because of this change :)

[javierhonduco]

Just checked that measured-rails, active-shipping with gem 'measured', github: 'Shopify/measured', ref: 'e2373afc6e14d085cbde1de1977f377e83855712' in the Gemfile and everything seems to work as with the current version 🎉