search

ChainSafe / fixed-sized-numbers-ts

Fixed Sized BigNumbers in TypeScript
2 stars 4 forks source link

Standardized instatiation value types #9

Closed ansermino closed 5 years ago

ansermino commented 5 years ago

By removing the ability to pass primitive numbers into the factory constructors we simplify our API and prevent misuse. Primitives can still be used by instantiating a BigNumber that is then passed in.

This PR also adds the ability to instantiate 54bit+ numbers with BigNumbers as BNs can handle arbitrary sized values.

Currently working a complete refactoring of the test suite so I have not updated the tests and thus CI will fail. Refactoring PR should be opened by end of day.

ansermino commented 5 years ago

@JonathanLorimer Please review and share your thoughts.

JonathanLorimer commented 5 years ago

@ansermino I think this is a good idea as well. The only comment I have is that if the user constructs a BigNumber with a Number that is larger than 53 bytes the value of the BigNumber will reflect the imprecision of JS Numbers. However, I think it is safe to assume that the user knows what the value of the BigNumber they are constructing is. Just want to make sure you are aware of that.

And we need to change some of the basic tests from Numbers to Strings before merging

ansermino commented 5 years ago

@JonathanLorimer thank you for pointing that out, I guess you could say I'm shifting the responsibility away from his library and into the BigNumber library. I have updated the tests in #10 I believe.

@GregTheGreek working on david/doc-improvements which should handle all updates necessary.