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

STV last round use "Final Round Surplus" rather than inactive #884

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 51 additions & 13 deletions src/main/java/network/brightspots/rcv/CastVoteRecord.java
Original file line number Diff line number Diff line change
Expand Up @@ -235,28 +235,66 @@ Map<String, BigDecimal> getWinnerToFractionalValue() {
// as far as tabulation is concerned, all that matters is whether
// it is active or not.
enum StatusForRound {
ACTIVE(false, "active"),
DID_NOT_RANK_ANY_CANDIDATES(true, "did not rank any candidates"),
EXHAUSTED_CHOICE(true, "exhausted choice"),
INVALIDATED_BY_OVERVOTE(true, "invalidated by overvote"),
INVALIDATED_BY_SKIPPED_RANKING(true, "invalidated by skipped ranking"),
INVALIDATED_BY_REPEATED_RANKING(true, "invalidated by repeated ranking"),
FINAL_ROUND_SURPLUS(false, "final round surplus");
ACTIVE(
false,
"Active",
"active"
),
DID_NOT_RANK_ANY_CANDIDATES(
true,
"Did Not Rank Any Candidates",
"didNotRankAnyCandidates"
),
EXHAUSTED_CHOICE(
true,
"Inactive Ballots by Exhausted Choices",
"exhaustedChoices"
),
INVALIDATED_BY_OVERVOTE(
true,
"Inactive Ballots by Overvotes",
"overvotes"
),
INVALIDATED_BY_SKIPPED_RANKING(
true,
"Inactive Ballots by Skipped Rankings",
"skippedRankings"
),
INVALIDATED_BY_REPEATED_RANKING(
true,
"Inactive Ballots by Repeated Rankings",
"repeatedRankings"
),
FINAL_ROUND_SURPLUS(
false,
"Final Round Surplus",
"finalRoundSurplus"
);

private final boolean isInactiveBallot;
private final String plaintext;

StatusForRound(boolean isInactiveBallot, String plaintext) {
private final String titleCaseKey;
private final String camelCaseKey;

StatusForRound(
boolean isInactiveBallot,
String titleCaseKey,
String camelCaseKey
) {
this.isInactiveBallot = isInactiveBallot;
this.plaintext = plaintext;
this.titleCaseKey = titleCaseKey;
this.camelCaseKey = camelCaseKey;
}

public boolean isInactiveBallot() {
return isInactiveBallot;
}

public String getPlaintext() {
return plaintext;
public String getTitleCaseKey() {
return titleCaseKey;
}

public String getCamelCaseKey() {
return camelCaseKey;
}
}

Expand Down
47 changes: 19 additions & 28 deletions src/main/java/network/brightspots/rcv/ResultsWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -382,20 +382,16 @@ private void generateSummarySpreadsheet(
csvPrinter.println();
}

List<Pair<String, StatusForRound>> statusesToPrint = new ArrayList<>();
statusesToPrint.add(new Pair<>("Overvotes",
StatusForRound.INVALIDATED_BY_OVERVOTE));
statusesToPrint.add(new Pair<>("Skipped Rankings",
StatusForRound.INVALIDATED_BY_SKIPPED_RANKING));
statusesToPrint.add(new Pair<>("Exhausted Choices",
StatusForRound.EXHAUSTED_CHOICE));
statusesToPrint.add(new Pair<>("Repeated Rankings",
StatusForRound.INVALIDATED_BY_REPEATED_RANKING));

for (Pair<String, StatusForRound> statusToPrint : statusesToPrint) {
csvPrinter.print("Inactive Ballots by " + statusToPrint.getKey());

StatusForRound status = statusToPrint.getValue();
List<StatusForRound> statusesToPrint = new ArrayList<>();
statusesToPrint.add(StatusForRound.INVALIDATED_BY_OVERVOTE);
statusesToPrint.add(StatusForRound.INVALIDATED_BY_SKIPPED_RANKING);
statusesToPrint.add(StatusForRound.EXHAUSTED_CHOICE);
statusesToPrint.add(StatusForRound.INVALIDATED_BY_REPEATED_RANKING);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to leave one more comment on this, but I want to be clear that I'm not requesting additional changes in this PR. It's easy to start pulling on a thread and not know where to stop! If I were responsible for this code, I would move this list to a static final field on the class:

private static final List<StatusForRound> STATUSES_TO_PRINT = List.of(
        StatusForRound.INVALIDATED_BY_OVERVOTE,   
        StatusForRound.INVALIDATED_BY_SKIPPED_RANKING, 
        StatusForRound.INVALIDATED_BY_SKIPPED_RANKING, 
        StatusForRound.INVALIDATED_BY_SKIPPED_RANKING);

And then I would reuse the same list starting in line 1027.

List.of() returns an ImmutableList, so you can't conditionally add the FINAL_ROUND_SURPLUS to it. I'll leave a second comment on that section of the code with additional thoughts.


for (StatusForRound statusToPrint : statusesToPrint) {
csvPrinter.print(statusToPrint.getTitleCaseKey());

StatusForRound status = statusToPrint;
for (int round = 1; round <= numRounds; round++) {
BigDecimal thisRoundInactive = roundTallies.get(round).getBallotStatusTally(status);
csvPrinter.print(thisRoundInactive);
Expand Down Expand Up @@ -457,7 +453,7 @@ private void generateSummarySpreadsheet(

if (config.usesSurpluses()) {
// row for final round surplus (if needed)
csvPrinter.print("Final Round Surplus");
csvPrinter.print(StatusForRound.FINAL_ROUND_SURPLUS.getTitleCaseKey());
for (int round = 1; round <= numRounds; round++) {
BigDecimal finalRoundSurplus =
roundTallies.get(round).getBallotStatusTally(StatusForRound.FINAL_ROUND_SURPLUS);
Expand Down Expand Up @@ -1028,23 +1024,18 @@ private Map<String, BigDecimal> updateCandidateNamesInTally(RoundTally roundSumm

private Map<String, BigDecimal> getInactiveJsonMap(RoundTally roundTally) {
Map<String, BigDecimal> inactiveMap = new HashMap<>();
List<Pair<String, StatusForRound>> statusesToPrint = new ArrayList<>();
statusesToPrint.add(new Pair<>("overvotes",
StatusForRound.INVALIDATED_BY_OVERVOTE));
statusesToPrint.add(new Pair<>("skippedRankings",
StatusForRound.INVALIDATED_BY_SKIPPED_RANKING));
statusesToPrint.add(new Pair<>("exhaustedChoices",
StatusForRound.EXHAUSTED_CHOICE));
statusesToPrint.add(new Pair<>("repeatedRankings",
StatusForRound.INVALIDATED_BY_REPEATED_RANKING));
List<StatusForRound> statusesToPrint = new ArrayList<>();
statusesToPrint.add(StatusForRound.INVALIDATED_BY_OVERVOTE);
statusesToPrint.add(StatusForRound.INVALIDATED_BY_SKIPPED_RANKING);
statusesToPrint.add(StatusForRound.EXHAUSTED_CHOICE);
statusesToPrint.add(StatusForRound.INVALIDATED_BY_REPEATED_RANKING);
if (config.usesSurpluses()
&& roundTally.getRoundNumber() == numRounds) {
statusesToPrint.add(new Pair<>("finalRoundSurplus",
StatusForRound.FINAL_ROUND_SURPLUS));
statusesToPrint.add(StatusForRound.FINAL_ROUND_SURPLUS);
}
for (Pair<String, StatusForRound> statusToPrint : statusesToPrint) {
for (StatusForRound statusToPrint : statusesToPrint) {
inactiveMap.put(
statusToPrint.getKey(), roundTally.getBallotStatusTally(statusToPrint.getValue()));
statusToPrint.getCamelCaseKey(), roundTally.getBallotStatusTally(statusToPrint));
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that you didn't love the stream suggestion! Since you're using a modifiable map here, you could use the static final immutable list described above like this:

private Map<String, BigDecimal> getInactiveJsonMap(RoundTally roundTally) {
    Map<String, BigDecimal> result = new HashMap<>();
    for (StatusForRound statusToPrint : STATUSES_TO_PRINT) {
        result.put(statusToPrint.getCamelCaseKey(), roundTally.getBallotStatusTally(statusToPrint));
    }

    if (config.usesSurpluses() && roundTally.getRoundNumber() == numRounds) {
        result.put(FINAL_ROUND_SURPLUS.getCamelCaseKey(), roundTally.getBallotStatusTally(FINAL_ROUND_SURPLUS));
    }

    return result;
}

Again, not asking for changes in this PR! Just pointing out what I see :) If it's useful to you, great! Overall, I think the changes that have already been made are a step in the right direction.

}
return inactiveMap;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/network/brightspots/rcv/Tabulator.java
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ private void recordSelectionForCastVoteRecord(

final String outcomeDescription = statusForRound == StatusForRound.ACTIVE
? selectedCandidate
: statusForRound.getPlaintext() + additionalLogText;
: statusForRound.getTitleCaseKey() + additionalLogText;
final VoteOutcomeType outcomeType =
selectedCandidate == null ? VoteOutcomeType.EXHAUSTED : VoteOutcomeType.COUNTED;
cvr.logRoundOutcome(
Expand Down
Loading