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

fix: use method of largest remainder to count poll votes #13554

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Oct 15, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
image image
image image
image image
image image
image image

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Quota methods is a good solution, yet I'd recommend to simplify its implementation for a better understanding

    if (!total) {
        return votes
    }

    const { wholes, remainders } = votes.reduce((acc: VoteAccumulator, vote) => {
        const quota = vote / total * 100
        acc.wholes.push(Math.floor(quota))
        acc.remainders.push(quota % 1)
        return acc
    }, { wholes: [], remainders: [] })

    // Calculate the sum of whole parts
    const sumWholes = wholes.reduce((acc, value) => acc + value, 0)

    // If the sum of whole parts is already 100, return it
    if (sumWholes === 100) {
        return wholes
    }

    // Calculate how many more votes we need to reach 100
    let remainingVotes = 100 - sumWholes

    // Get the indexes of the largest remainders
    const largestRemainders = remainders
        .map((value, index) => ({ value, index }))
        .sort((a, b) => b.value - a.value)
        .slice(0, remainingVotes)
        .map(item => item.index)

    // Distribute the remaining votes
    for (const index of largestRemainders) {
        wholes[index]++
    }

    return wholes

src/components/PollViewer/PollViewer.vue Outdated Show resolved Hide resolved
@Antreesy
Copy link
Contributor Author

Antreesy commented Oct 28, 2024

// If the sum of whole parts is already 100, return it

Almost never the case, unless you have exactly 1,2,4,5,10,20,50 or 100 votes. Math.round is better here
But since Math.round is almost always the case, maybe I can move some calculations after this return

// Distribute the remaining votes

That rounds until 100%, but that's exactly the possible error for equal remainders described in the issue:
image

@DorraJaouad

This comment was marked as off-topic.

Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Tested, we agreed to prioritize fair accuracy than mainly targeting 100%.

@Antreesy
Copy link
Contributor Author

/backport to stable30

@Antreesy Antreesy merged commit 089172e into main Oct 29, 2024
46 checks passed
@Antreesy Antreesy deleted the fix/11867/poll-rounding branch October 29, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poll rounding error
2 participants