ericelliott/moneysafe

Sections of README are out-dated

Closed this issue · 4 comments

The following sections of the README are out-dated (apply to v1 and not v2) and should either be removed or corrected:

  1. How does Money$afe work? https://github.com/ericelliott/moneysafe/blob/97726a87a7020402b488452c3d443b13dfda263b/README.md#how-does-moneyafe-work

    It mentions "storing and acting on the amounts in cents", which is no longer accurate. I suggest we either remove this section entirely or update it to describe the use of bignumber.js instead.

  2. $(dollars) => Money https://github.com/ericelliott/moneysafe/blob/97726a87a7020402b488452c3d443b13dfda263b/README.md#dollars--money

    The Example uses .cents, which no longer works.

    The next section says "The resulting value will be in cents", which is no longer correct.

  3. in$ Utility https://github.com/ericelliott/moneysafe/blob/97726a87a7020402b488452c3d443b13dfda263b/README.md#in-utility

    This utility no longer exists. This section should be removed.

    Perhaps, in its place, we should add some advice for how to migrate from v1 to v2.

Thank you! I've tried to update this several times but apparently, I missed some things. Would you like to take a crack at a PR?

I'll take a crack at it, but how do you recommend rounding a Money type to its nearest supported significant digit (after arithmetic operations are complete) now that the in$ utility is gone?

Would you recommend one of the following or something else?

  1. Using built-in .toFixed() with Number coercion:

    +$(-45).div(99).toFixed() // -0.45
  2. Using your own version of round_to_precision from MDN:

    function round_to_precision(x, precision) {
      var y = +x + (precision === undefined ? 0.5 : precision/2);
      return y - (y % (precision === undefined ? 1 : +precision));
    }
    
    round_to_precision($(-45).div(99).valueOf(), 0.01) // -0.44
  3. Using your own version of this StackOverflow answer:

    function roundTo (num, precision) {
      return +(Math.round(num + ('e+' + precision))  + ('e-' + precision));
    }
    
    roundTo($(-45).div(99).valueOf(), 2) // -0.45

What do you think about building in some convenience for this into the Money type itself, so that we can leverage the desired number of decimals for a particular currency?

I think option 1 makes the most sense, because we can't assume that it's safe to simply try to convert any monesafe object to a string. Option 1 forces the user to consider the impact of converting to a number.

Ok, that's what I suggested in docs in #35