BrightSpots / rcv

Ranked Choice Voting Universal Tabulator
Mozilla Public License 2.0
73 stars 19 forks source link

Refactor how "Tabulate By Slice" works #874

Open artoonie opened 3 months ago

artoonie commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

I think we're on the same page here with bullet point (1) -- RoundTally could know about slices and force the caller to provide slice-level data where it's required.

Message ID: @.***>