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

Refactor how "Tabulate By Slice" works #874

Open
artoonie opened this issue Jul 25, 2024 · 2 comments
Open

Refactor how "Tabulate By Slice" works #874

artoonie opened this issue Jul 25, 2024 · 2 comments

Comments

@artoonie
Copy link
Collaborator

The "Tabulate by Slice" code works by keeping entirely separate structures throughout the tabulation process for by-slice versus whole-contest results.

This separation requires every call to a whole-contest structure to be followed by a call to the per-slice structure.

That manual process is error-prone, hard to test, and makes the code a little sloppy.

I recommend we refactor this. The specifics TBD, but some ideas:

  1. Methods that apply to both should take require providing the per-slice structures too, so it's impossible to call them without providing a slice (e.g. RoundTally.addX(RoundTallies, x)
  2. We have a helper class which does essentially (1) but outside of the class itself (e.g. addX(RoundTally wholeContest, RoundTallies bySlices)
  3. We generalize the whole contest idea into a special case of a Slice, so we don't treat the whole contest any differently than a slice without some explicit effort

I lean towards (3).

@yezr
Copy link
Collaborator

yezr commented Aug 23, 2024

testing some slice level tally stuff and thinking about this. What if RoundTally was made aware of tabulation level slices when it was initialized and all the current methods that update tallies would fill in slice tallies based on what was initialized. I don't have all the context for coupling and what is unhealthy code practices but if RoundTally knew about the slices it could be made responsible for filling them in.

@artoonie
Copy link
Collaborator Author

artoonie commented Aug 23, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants