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

Round totals rather than expense by expense #88

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

justcallmelarry
Copy link
Contributor

@justcallmelarry justcallmelarry commented Feb 9, 2024

Problem statement

As raised in #85, balances get a bit weird when they are not equally divisible between all participants.

Current logic does rounding on an expense by expense basis, which ends up with some inconsistencies. I made three groups with three users each, which I then added ten $10 expenses to, and wound up with the following results:

Images Screenshot 2024-02-09 at 16 19 59 Screenshot 2024-02-09 at 16 21 34 Screenshot 2024-02-09 at 16 22 59

So it's not only about who pays for expenses, but also the order of the users has an impact on who pays what (poor, poor Jack…).
This seems inherently incorrect/unfair, so I set out to try to fix the calculations.

Proposed solution 1

Since there is nothing wrong with the general setup of calculating the balances, I just tried to change the rounding parts and leave the rest as is.
Do not do any rounding when calculating balances, instead only do rounding once the totals for each user has been calculated.

Issues with the solution

I noticed that sometimes the balances got offset by 0.01 (+16.67 / -16.66), which looked ugly (and perhaps confusing). Furthermore, when all the reimbursements were done, the extra 0.01 was still left as a positive balance, even though no reimbursements were listed.

Proposed solution 2 (the one implemented in this PR)

Instead of using the balances as they are, use them to calculate the reimbursements just like before, but then construct something I called "public balances" from those reimbursements, making sure that the balances will always show the same amounts as the reimbursements

Issues with the solution

Another round trip over the reimbursements/balances is needed. While this is not a huge operation, it does scale with users, so very large groups needs a bit more processing.
I have a hard time seeing groups with thousands of users, but maybe they do exist.

Issues

Closes #85 when merged

Copy link
Member

@scastiel scastiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love your changes in the algorithm. To be honest I can’t promise we’ll never find some cases where the result isn’t what we expect, but it’s definitely worth trying :)

To answer your concern, I can’t image groups with so many participants that scaling it becomes an issue ;)

@scastiel scastiel merged commit f7a13a0 into spliit-app:main Feb 13, 2024
1 check passed
@justcallmelarry justcallmelarry deleted the feature/round-totals-only branch February 13, 2024 20:02
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.

Wrong / unfair rounding
2 participants