From e3c51f3c0147a0ea7efb5350d92ea8ea40f7c393 Mon Sep 17 00:00:00 2001 From: domi41 Date: Thu, 17 Jun 2021 09:54:57 +0200 Subject: [PATCH] feat: raise a proper exception on invalid tallies Thanks to Paris' Lutece team for spotting this pitfall full of spikes right behind the door. --- README.md | 27 +------ .../mieuxvoter/mj/DeliberatorInterface.java | 2 +- .../mj/MajorityJudgmentDeliberator.java | 47 +++++++++++- ...aultGrade.java => StaticDefaultTally.java} | 10 +-- .../mj/MajorityJudgmentDeliberatorTest.java | 75 +++++++++++++++---- 5 files changed, 115 insertions(+), 46 deletions(-) rename src/main/java/fr/mieuxvoter/mj/{TallyWithDefaultGrade.java => StaticDefaultTally.java} (65%) diff --git a/README.md b/README.md index 0f0c9d1..ca9ebca 100644 --- a/README.md +++ b/README.md @@ -59,12 +59,12 @@ Got even more than that ? Use `BigInteger`s ! ### Using a static default grade -Want to set a static default grade ? Use a `TallyWithDefaultGrade` instead of a `Tally`. +Want to set a static default grade ? Use a `StaticDefaultTally` instead of a `Tally`. ```java Integer amountOfJudges = 18; Integer defaultGrade = 0; // "worst" grade (usually "to reject") -TallyInterface tally = new TallyWithDefaultGrade(new ProposalTallyInterface[] { +TallyInterface tally = new StaticDefaultTally(new ProposalTallyInterface[] { // Amounts of judgments received of each grade, from "worst" grade to "best" grade new ProposalTally(new Integer[]{4, 5, 2, 1, 3, 1, 2}), // Proposal A new ProposalTally(new Integer[]{3, 6, 2, 1, 3, 1, 2}), // Proposal B @@ -131,29 +131,6 @@ ResultInterface result = mj.deliberate(tally); ``` -## Roadmap - -- [x] Unit-Tests -- [x] Deliberation algorithm - - [x] Tally Analysis - - [x] Score Calculus - - [x] Ranking -- [x] Release v0.1.0 -- [x] Guess the amount of judges -- [x] Allow defining a default grade - - [x] Static Grade (configurable) - - [x] Normalization (using Least Common Multiple) - - [x] Median Grade -- [ ] Release v0.2.0 -- [ ] Publish on package repositories - - [ ] Gradle - - [ ] Maven - - [ ] … ? (please share your knowledge to help us!) -- [ ] Release v0.3.0 -- [ ] Use it somewhere in an application, adjust API as needed (one last time) -- [ ] Release v1.0.0 - - ## Gondor calls for Help! We are not accustomed to Java library development and we'd love reviews from seasoned veterans ! diff --git a/src/main/java/fr/mieuxvoter/mj/DeliberatorInterface.java b/src/main/java/fr/mieuxvoter/mj/DeliberatorInterface.java index 25e0a64..bf080f5 100644 --- a/src/main/java/fr/mieuxvoter/mj/DeliberatorInterface.java +++ b/src/main/java/fr/mieuxvoter/mj/DeliberatorInterface.java @@ -18,6 +18,6 @@ package fr.mieuxvoter.mj; */ public interface DeliberatorInterface { - public ResultInterface deliberate(TallyInterface tally); + public ResultInterface deliberate(TallyInterface tally) throws InvalidTallyException; } diff --git a/src/main/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberator.java b/src/main/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberator.java index 6092105..bcc510c 100644 --- a/src/main/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberator.java +++ b/src/main/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberator.java @@ -38,7 +38,9 @@ final public class MajorityJudgmentDeliberator implements DeliberatorInterface { } @Override - public ResultInterface deliberate(TallyInterface tally) { + public ResultInterface deliberate(TallyInterface tally) throws InvalidTallyException { + checkTally(tally); + ProposalTallyInterface[] tallies = tally.getProposalsTallies(); BigInteger amountOfJudges = tally.getAmountOfJudges(); Integer amountOfProposals = tally.getAmountOfProposals(); @@ -86,7 +88,50 @@ final public class MajorityJudgmentDeliberator implements DeliberatorInterface { result.setProposalResults(proposalResults); return result; } + + protected void checkTally(TallyInterface tally) throws UnbalancedTallyException { + if ( ! isTallyCoherent(tally)) { + throw new IncoherentTallyException(); + } + if ( ! isTallyBalanced(tally)) { + throw new UnbalancedTallyException(); + } + } + + protected boolean isTallyCoherent(TallyInterface tally) { + boolean coherent = true; + for (ProposalTallyInterface proposalTally : tally.getProposalsTallies()) { + for (BigInteger gradeTally : proposalTally.getTally()) { + if (-1 == gradeTally.compareTo(BigInteger.ZERO)) { + coherent = false; // negative tallies are not coherent + } + } + } + + return coherent; + } + protected boolean isTallyBalanced(TallyInterface tally) { + boolean balanced = true; + BigInteger amountOfJudges = BigInteger.ZERO; + boolean firstProposal = true; + for (ProposalTallyInterface proposalTally : tally.getProposalsTallies()) { + if (firstProposal) { + amountOfJudges = proposalTally.getAmountOfJudgments(); + firstProposal = false; + } else { + if (0 != amountOfJudges.compareTo(proposalTally.getAmountOfJudgments())) { + balanced = false; + } + } + } + + return balanced; + } + + /** + * @see computeScore() below + */ protected String computeScore(ProposalTallyInterface tally, BigInteger amountOfJudges) { return computeScore(tally, amountOfJudges, this.favorContestation, this.numerizeScore); } diff --git a/src/main/java/fr/mieuxvoter/mj/TallyWithDefaultGrade.java b/src/main/java/fr/mieuxvoter/mj/StaticDefaultTally.java similarity index 65% rename from src/main/java/fr/mieuxvoter/mj/TallyWithDefaultGrade.java rename to src/main/java/fr/mieuxvoter/mj/StaticDefaultTally.java index 8cdd93a..e750b5e 100644 --- a/src/main/java/fr/mieuxvoter/mj/TallyWithDefaultGrade.java +++ b/src/main/java/fr/mieuxvoter/mj/StaticDefaultTally.java @@ -2,7 +2,7 @@ package fr.mieuxvoter.mj; import java.math.BigInteger; -public class TallyWithDefaultGrade extends DefaultGradeTally implements TallyInterface { +public class StaticDefaultTally extends DefaultGradeTally implements TallyInterface { /** * Grades are represented as numbers, as indices in a list. @@ -18,25 +18,25 @@ public class TallyWithDefaultGrade extends DefaultGradeTally implements TallyInt */ protected Integer defaultGrade = 0; - public TallyWithDefaultGrade(TallyInterface tally, Integer defaultGrade) { + public StaticDefaultTally(TallyInterface tally, Integer defaultGrade) { super(tally.getProposalsTallies(), tally.getAmountOfJudges()); this.defaultGrade = defaultGrade; fillWithDefaultGrade(); } - public TallyWithDefaultGrade(ProposalTallyInterface[] proposalsTallies, BigInteger amountOfJudges, Integer defaultGrade) { + public StaticDefaultTally(ProposalTallyInterface[] proposalsTallies, BigInteger amountOfJudges, Integer defaultGrade) { super(proposalsTallies, amountOfJudges); this.defaultGrade = defaultGrade; fillWithDefaultGrade(); } - public TallyWithDefaultGrade(ProposalTallyInterface[] proposalsTallies, Long amountOfJudges, Integer defaultGrade) { + public StaticDefaultTally(ProposalTallyInterface[] proposalsTallies, Long amountOfJudges, Integer defaultGrade) { super(proposalsTallies, amountOfJudges); this.defaultGrade = defaultGrade; fillWithDefaultGrade(); } - public TallyWithDefaultGrade(ProposalTallyInterface[] proposalsTallies, Integer amountOfJudges, Integer defaultGrade) { + public StaticDefaultTally(ProposalTallyInterface[] proposalsTallies, Integer amountOfJudges, Integer defaultGrade) { super(proposalsTallies, amountOfJudges); this.defaultGrade = defaultGrade; fillWithDefaultGrade(); diff --git a/src/test/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberatorTest.java b/src/test/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberatorTest.java index 473b6f0..3dffee7 100644 --- a/src/test/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberatorTest.java +++ b/src/test/java/fr/mieuxvoter/mj/MajorityJudgmentDeliberatorTest.java @@ -1,5 +1,6 @@ package fr.mieuxvoter.mj; +import static org.junit.Assert.assertThrows; import static org.junit.jupiter.api.Assertions.*; import java.math.BigInteger; @@ -8,6 +9,7 @@ import javax.json.JsonArray; import javax.json.JsonObject; import javax.json.JsonValue; +import org.junit.function.ThrowingRunnable; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -19,7 +21,7 @@ class MajorityJudgmentDeliberatorTest { @DisplayName("Test majority judgment deliberation from JSON assertions") @ParameterizedTest(name="#{index} {0}") @JsonFileSource(resources = "/assertions.json") - public void testFromJson(JsonObject datum) { + public void testFromJson(JsonObject datum) throws Throwable { // This test uses the JSON file in test/resources/ // It also allows testing the various modes of default grades. @@ -41,7 +43,7 @@ class MajorityJudgmentDeliberatorTest { String mode = datum.getString("mode", "None"); TallyInterface tally; if ("StaticDefault".equalsIgnoreCase(mode)) { - tally = new TallyWithDefaultGrade(tallies, amountOfParticipants, datum.getInt("default", 0)); + tally = new StaticDefaultTally(tallies, amountOfParticipants, datum.getInt("default", 0)); } else if ("MedianDefault".equalsIgnoreCase(mode)) { tally = new MedianDefaultTally(tallies, amountOfParticipants); } else if ("Normalized".equalsIgnoreCase(mode)) { @@ -66,7 +68,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test the basic demo usage of the README") - public void testDemoUsage() { + public void testDemoUsage() throws Throwable { DeliberatorInterface mj = new MajorityJudgmentDeliberator(); TallyInterface tally = new Tally(new ProposalTallyInterface[] { new ProposalTally(new Integer[]{4, 5, 2, 1, 3, 1, 2}), @@ -83,7 +85,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test the basic demo usage with billions of participants") - public void testDemoUsageWithBigNumbers() { + public void testDemoUsageWithBigNumbers() throws Throwable { DeliberatorInterface mj = new MajorityJudgmentDeliberator(); TallyInterface tally = new Tally(new ProposalTallyInterface[] { new ProposalTally(new Long[]{11312415004L, 21153652410L, 24101523299L, 18758623562L}), @@ -103,7 +105,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test the collect demo usage of the README") - public void testDemoUsageCollectedTally() { + public void testDemoUsageCollectedTally() throws Throwable { Integer amountOfProposals = 3; Integer amountOfGrades = 4; DeliberatorInterface mj = new MajorityJudgmentDeliberator(); @@ -155,7 +157,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test the normalized collect demo usage of the README") - public void testDemoUsageNormalizedCollectedTally() { + public void testDemoUsageNormalizedCollectedTally() throws Throwable { Integer amountOfProposals = 4; Integer amountOfGrades = 3; DeliberatorInterface mj = new MajorityJudgmentDeliberator(); @@ -198,11 +200,11 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test with a static default grade (\"worst grade\" == 0)") - public void testWithStaticDefaultGrade() { + public void testWithStaticDefaultGrade() throws Throwable { DeliberatorInterface mj = new MajorityJudgmentDeliberator(); Long amountOfJudges = 3L; Integer defaultGrade = 0; - TallyInterface tally = new TallyWithDefaultGrade(new ProposalTallyInterface[] { + TallyInterface tally = new StaticDefaultTally(new ProposalTallyInterface[] { new ProposalTally(new Integer[]{ 0, 0, 1 }), new ProposalTally(new Integer[]{ 0, 3, 0 }), new ProposalTally(new Integer[]{ 2, 0, 1 }), @@ -219,7 +221,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test static default grade with thousands of proposals and millions of judges") - public void testStaticDefaultWithThousandsOfProposals() { + public void testStaticDefaultWithThousandsOfProposals() throws Throwable { int amountOfProposals = 1337; Integer amountOfJudges = 60000000; Integer defaultGrade = 0; @@ -228,7 +230,7 @@ class MajorityJudgmentDeliberatorTest { for (int i = 0 ; i < amountOfProposals ; i++) { tallies[i] = new ProposalTally(new Integer[]{ 7, 204, 107 }); } - TallyInterface tally = new TallyWithDefaultGrade(tallies, amountOfJudges, defaultGrade); + TallyInterface tally = new StaticDefaultTally(tallies, amountOfJudges, defaultGrade); ResultInterface result = mj.deliberate(tally); @@ -264,7 +266,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test with a median default grade") - public void testMedianDefaultGrade() { + public void testMedianDefaultGrade() throws Throwable { Integer amountOfJudges = 42; DeliberatorInterface mj = new MajorityJudgmentDeliberator(); TallyInterface tally = new MedianDefaultTally(new ProposalTallyInterface[] { @@ -288,7 +290,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test normalized tallies with thousands of (prime) proposals") - public void testNormalizedWithThousandsOfPrimeProposals() { + public void testNormalizedWithThousandsOfPrimeProposals() throws Throwable { // We're using primes to test the upper bounds of our LCM shenanigans. // This test takes a long time! (3 seconds) @@ -316,7 +318,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test normalized tallies with thousands of proposals") - public void testNormalizedWithThousandsOfProposals() { + public void testNormalizedWithThousandsOfProposals() throws Throwable { // This test is faster than the primes one (0.4 seconds), // since primes are the worst-case scenario for our LCM. @@ -343,7 +345,7 @@ class MajorityJudgmentDeliberatorTest { @Test @DisplayName("Test favoring adhesion") - public void testFavoringAdhesion() { + public void testFavoringAdhesion() throws Exception { boolean favorContestation = false; Integer amountOfJudges = 4; DeliberatorInterface mj = new MajorityJudgmentDeliberator(favorContestation); @@ -365,6 +367,51 @@ class MajorityJudgmentDeliberatorTest { assertEquals(1, result.getProposalResults()[2].getAnalysis().getMedianGrade()); } + @Test + @DisplayName("Fail on unbalanced tallies") + public void testFailureOnUnbalancedTallies() { + Integer amountOfJudges = 2; + DeliberatorInterface mj = new MajorityJudgmentDeliberator(); + TallyInterface tally = new Tally(new ProposalTallyInterface[] { + new ProposalTally(new Integer[]{ 0, 0, 2 }), + new ProposalTally(new Integer[]{ 0, 1, 0 }), + new ProposalTally(new Integer[]{ 2, 0, 0 }), + }, amountOfJudges); + + assertThrows( + "An exception is raised", + UnbalancedTallyException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + mj.deliberate(tally); + } + } + ); + } + + @Test + @DisplayName("Fail on negative tallies") + public void testFailureOnNegativeTallies() { + Integer amountOfJudges = 2; + DeliberatorInterface mj = new MajorityJudgmentDeliberator(); + TallyInterface tally = new Tally(new ProposalTallyInterface[] { + new ProposalTally(new Integer[]{ 0, 4, -2 }), + new ProposalTally(new Integer[]{ 0, 2, 0 }), + }, amountOfJudges); + + assertThrows( + "An exception is raised", + IncoherentTallyException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + mj.deliberate(tally); + } + } + ); + } + // …