Skip to content
New issue

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

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

Already on GitHub? # to your account

Add floor and ceiling to FixedNumber #1037

Closed
thegostep opened this issue Sep 7, 2020 · 10 comments
Closed

Add floor and ceiling to FixedNumber #1037

thegostep opened this issue Sep 7, 2020 · 10 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@thegostep
Copy link

No description provided.

@ricmoo
Copy link
Member

ricmoo commented Sep 7, 2020

An important reason why ethers has its own implementation of BigNumber is to be immutable.

So no in-place operations are available...

It makes things much simpler throughout the async nature of blockchain that capturing a reference is sufficient (also copying is fast; just store a reference) which requires all numbers remain immutable. Sorry. :s

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Sep 7, 2020
@thegostep
Copy link
Author

Is there an equivalent workaround with existing methods?

@ricmoo
Copy link
Member

ricmoo commented Sep 8, 2020

Well, you can just re-assign to the original value. :)

// equivalent to value.idiv(divisor)
value = value.div(divisor);

@thegostep
Copy link
Author

Ah - okay sorry, I was not referring to inplace division, I was referring to integer division: https://mikemcl.github.io/bignumber.js/#divInt

@ricmoo
Copy link
Member

ricmoo commented Sep 9, 2020

Oh, I see. That makes more sense. :)

The BigNumber class will likely never support that, however, you can use the FixedNumber class to get something similar:

x = FixedNumber.from("5");
y = FixedNumber.from("0.7");

quotient = x.divUnsafe(y));
// "7.142857142857142857"

quotient.round()
// "7.0"

You can also specify the internal fixed format to use, which is by default fixed128x18 (i.e. 128 bits wide, representing 18 decimals).

Does that help?

@thegostep
Copy link
Author

bare with me - I am a math noob

Is is possible to have a way for specifying if division rounds up or down?

@ricmoo
Copy link
Member

ricmoo commented Sep 10, 2020

Oh! That is actually something I’ve been meaning to add, but it hasn’t been a priority. But I can add it this week.

I plan to support a bunch of tie-breaking rounding algorithms:

  • round half up (this is what round currently does)
  • round up (ceiling)
  • round down (floor)
  • round toward zero
  • round away from zero
  • round to odd
  • round to even

For now I will add floor and ceiling. If there is any other rounding strategy you need though, let me know.

@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. and removed discussion Questions, feedback and general information. labels Sep 10, 2020
@ricmoo ricmoo changed the title add idiv method to bignumber implementation Add floor and ceiling to FixedNumber Sep 10, 2020
@ricmoo
Copy link
Member

ricmoo commented Sep 12, 2020

I've added a .floor and .ceiling method to BigNumber in 5.0.13. Try it out and let me know if it works for you.

Thanks! :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Sep 12, 2020
@ricmoo
Copy link
Member

ricmoo commented Sep 13, 2020

This has been published, so I'm going to close it now. If you have any issues though, please re-open.

Thanks! :)

@ricmoo ricmoo closed this as completed Sep 13, 2020
@thegostep
Copy link
Author

I've been trying to convert this function into using only ethers BigNumber - any tips?
image

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants