Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.5.0 update #7

Merged
merged 1 commit into from Nov 15, 2015
Merged

v0.5.0 update #7

merged 1 commit into from Nov 15, 2015

Conversation

caccialdo
Copy link
Contributor

Because of all the non-backward compatible changes listed below, I figured this update should be treated as a major update.

Changes:

  • fromDecimal now accepts integer in case the float turns out to be an integer: Money.fromDecimal(10, Money.EUR) has an amount of 1000 cents.
  • fromDecimal now accepts decimals as strings: Money.fromDecimal("10.1", Money.EUR) has an amount of 1010 cents.
  • Fixed bug where multipliers[0] in fromDecimal method was 0. Now 1.
  • Add toJSON and toString methods to allow string concatenation and calls with JSON.stringify.
  • Add toDecimal method to extract the decimal amount as a float (cf. What about toDecimal? #6).
  • Allow optional second argument to divide and multiply method to allow for other functions than Math.round.
  • Update/add tests for new revision.
  • Update documentation.
  • Pin lodash dependency to 3.x.x to avoid any API breaking change.
  • Use latest node LTS v4.2.2 through the .nvmrc file.
  • Add .editorconfig file to improve code style consistency across editors.

toString method courtesy of @drewclauson

@davidkalosi
Copy link
Owner

Hi and thanks for the PR,

I have one big issue with it - the removal of the object construction method is a no go.
I am using this to create money objects stored in mongodb.
The static fromInteger method was introduced to have the following logic

  1. the constructor only accepts an the amont as integer + currency
  2. fromInteger can be passed either and object with an integer amount or 2 parameters
  3. fromDecimal is the same as as added bower.json file #2 just for decimals.

I am using the code

Money.fromInteger(varFromMongo);

Where the var is an object pulled from the db,

@caccialdo
Copy link
Contributor Author

Fair enough. My initial though here was that it was just creating a bit of confusion having both signature from a user perspective and from a maintainer perspective since it adds branches to the code. Happy to add the feature back in since the other features could be useful as well.

In the future, the lib could temporary be built as 2 bundles using something like webpack to give some time for users to update their code to the 2 arguments-only signature while the feature gets slowly deprecated. Just food for thought 👍

@caccialdo caccialdo changed the title v1 update v0.5.0 update Nov 14, 2015
@caccialdo
Copy link
Contributor Author

Updated the PR and added back support for the object constructor. Brought down the version bump to 0.5.0 as it doesn't break backward compatibility.

davidkalosi added a commit that referenced this pull request Nov 15, 2015
@davidkalosi davidkalosi merged commit d27f023 into davidkalosi:master Nov 15, 2015
@davidkalosi
Copy link
Owner

merged. thank you ;)

BTW I a considering to add a BigMoney class with the same prototype that could handle arbitrary-precision calculations.

@caccialdo caccialdo deleted the 1.0.0 branch November 15, 2015 14:14
@caccialdo
Copy link
Contributor Author

Do you mean, using something like https://github.com/MikeMcl/big.js ? The library is not crazy large (~2.8kB) but still has a non negligible weight compared to js-money.

@davidkalosi
Copy link
Owner

yeah something like that, 2.8kb is nothing theese days.
I need an implementation that won't be loosing precision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants