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

Panic on money.Allocate() #122

Closed
evilnoxx opened this issue Oct 28, 2022 · 3 comments
Closed

Panic on money.Allocate() #122

evilnoxx opened this issue Oct 28, 2022 · 3 comments

Comments

@evilnoxx
Copy link
Contributor

Hello,

The Allocate function can panic in the if the given ratios are all zero.
This makes sense, since the function will try to divide de given amount in parts of zero.

However, in my application it can make sense to try to allocate zero money, which would be the sum of some item prices, using those same item prices as ratios. Since the items add up to zero, because they are free, you get a call like

amount := money.New(0, money.EUR)
parties, err := amount.Allocate(0,0,0)

I wonder if, in this case, the Allocate function shouldn't return a slice filled with zeros with the same length as the given ratios, instead of panicking while dividing by zero.

On the other hand, Allocating is just another word for dividing, and zero divided by zero should be undefined, and not zero.

@DianaPinheiro
Copy link

The software must be prepared to deal with these types of situations instead of giving Panic. Returning a slice filled with zeros with the same length as the given ratios or a graceful error seems like good options

@Rhymond
Copy link
Owner

Rhymond commented Oct 31, 2022

I totally agree with @DianaPinheiro, the code must be deterministic.
I would expect zeroes should be returned on a given example. When you try to divide 0 cents to parties, everyone should get 0. @evilnoxx would you have time to work on the PR?

@evilnoxx
Copy link
Contributor Author

@Rhymond I gave it a shot. #123

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

No branches or pull requests

3 participants