From db09066ab91868f5c243b3be3cf6cb273f010ebf Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sat, 12 Mar 2022 12:02:28 +0500 Subject: [PATCH 01/23] tests: added some tests to reveal business cases --- .../java/org/elections/ElectionsTest.java | 154 ++++++++++++++---- 1 file changed, 120 insertions(+), 34 deletions(-) diff --git a/java/src/test/java/org/elections/ElectionsTest.java b/java/src/test/java/org/elections/ElectionsTest.java index 5c48f71..5dce6b1 100644 --- a/java/src/test/java/org/elections/ElectionsTest.java +++ b/java/src/test/java/org/elections/ElectionsTest.java @@ -1,55 +1,141 @@ package org.elections; +import java.util.stream.IntStream; import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import java.util.Arrays; import java.util.List; import java.util.Map; +@DisplayName("Тесты механизма голосования") class ElectionsTest { + private final Map> electorsByDistrict = Map.of( + "District 1", Arrays.asList("Bob", "Anna", "Jess", "July"), + "District 2", Arrays.asList("Jerry", "Simon"), + "District 3", Arrays.asList("Johnny", "Matt", "Carole") + ); - @Test - void electionWithoutDistricts() { - Map> list = Map.of( - "District 1", Arrays.asList("Bob", "Anna", "Jess", "July"), - "District 2", Arrays.asList("Jerry", "Simon"), - "District 3", Arrays.asList("Johnny", "Matt", "Carole") - ); - Elections elections = new Elections(list, false); - elections.addCandidate("Michel"); - elections.addCandidate("Jerry"); - elections.addCandidate("Johnny"); + @Nested + @DisplayName("Голосование без учета округов") + class NoDistrictsTest { - elections.voteFor("Bob", "Jerry", "District 1"); - elections.voteFor("Jerry", "Jerry", "District 2"); - elections.voteFor("Anna", "Johnny", "District 1"); - elections.voteFor("Johnny", "Johnny", "District 3"); - elections.voteFor("Matt", "Donald", "District 3"); - elections.voteFor("Jess", "Joe", "District 1"); - elections.voteFor("Simon", "", "District 2"); - elections.voteFor("Carole", "", "District 3"); + @Nested + @DisplayName("Список зарегистрированных кандидатов пуст") + class NoCandidatesTest { + @Test + @DisplayName("Голоса за любого кандидата дают 100% испорченных бюллетеней") + void votesForAnyCandidate_shouldShow100PercentNulls() { + Elections elections = new Elections(electorsByDistrict, false); + elections.voteFor("Bob", "Jerry", "District 1"); + elections.voteFor("Anna", "Jerry", "District 1"); - Map results = elections.results(); + Map results = elections.results(); - Map expectedResults = Map.of( - "Jerry", "50,00%", - "Johnny", "50,00%", - "Michel", "0,00%", - "Blank", "25,00%", - "Null", "25,00%", - "Abstention", "11,11%"); - Assertions.assertThat(results).isEqualTo(expectedResults); + Map expectedResults = Map.of( + "Blank", "0,00%", + "Null", "100,00%", + "Abstention", "77,78%"); + Assertions.assertThat(results).isEqualTo(expectedResults); + } + + @Test + @DisplayName("Голос от неизвестного избирателя учитывается и уменьшает неявку") + void oneVoteFromUnknownElector_shouldDecreaseAbstention() { + Elections elections = new Elections(electorsByDistrict, false); + elections.voteFor("Eugene", "Jerry", "District 1"); + + Map results = elections.results(); + + Map expectedResults = Map.of( + "Blank", "0,00%", + "Null", "100,00%", + "Abstention", "88,89%"); + Assertions.assertThat(results).isEqualTo(expectedResults); + } + + @Test + @DisplayName("Повторное голосование доступно и уменьшает неявку") + void multipleVotesFromSameElector_shouldDecreaseAbstention() { + Elections elections = new Elections(electorsByDistrict, false); + IntStream.range(1,10) + .forEach(i -> elections.voteFor("Eugene", "Jerry", "District 1")); + + Map results = elections.results(); + + Map expectedResults = Map.of( + "Blank", "0,00%", + "Null", "100,00%", + "Abstention", "0,00%"); + Assertions.assertThat(results).isEqualTo(expectedResults); + } + + @Test + @DisplayName("Если голосов больше, чем избирателей, неявка отрицательна") + void tooManyVotes_shouldMakeAbstentionNegative() { + Elections elections = new Elections(electorsByDistrict, false); + IntStream.range(1,11) + .forEach(i -> elections.voteFor("Eugene", "Jerry", "District 1")); + + Map results = elections.results(); + + Map expectedResults = Map.of( + "Blank", "0,00%", + "Null", "100,00%", + "Abstention", "-11,11%"); + Assertions.assertThat(results).isEqualTo(expectedResults); + + } + + } + + @Test + @DisplayName("Approval-Test для подсчета голосов") + void electionWithoutDistricts() { + Elections elections = new Elections(electorsByDistrict, false); + elections.addCandidate("Michel"); + elections.addCandidate("Jerry"); + elections.addCandidate("Johnny"); + + elections.voteFor("Bob", "Jerry", "District 1"); + elections.voteFor("Jerry", "Jerry", "District 2"); + elections.voteFor("Anna", "Johnny", "District 1"); + elections.voteFor("Johnny", "Johnny", "District 3"); + elections.voteFor("Matt", "Donald", "District 3"); + elections.voteFor("Jess", "Joe", "District 1"); + elections.voteFor("Simon", "", "District 2"); + elections.voteFor("Carole", "", "District 3"); + + Map results = elections.results(); + + Map expectedResults = Map.of( + "Jerry", "50,00%", + "Johnny", "50,00%", + "Michel", "0,00%", + "Blank", "25,00%", + "Null", "25,00%", + "Abstention", "11,11%"); + Assertions.assertThat(results).isEqualTo(expectedResults); + } + + } + + + + @Test + void givenNoCandidatesWithDistricts_oneVoteForAnyCandidate_shouldThrow() { + Elections elections = new Elections(electorsByDistrict, true); + elections.voteFor("Bob", "Jerry", "District 1"); + + Assertions.assertThatNullPointerException().isThrownBy(elections::results); } + @Test void electionWithDistricts() { - Map> list = Map.of( - "District 1", Arrays.asList("Bob", "Anna", "Jess", "July"), - "District 2", Arrays.asList("Jerry", "Simon"), - "District 3", Arrays.asList("Johnny", "Matt", "Carole") - ); - Elections elections = new Elections(list, true); + Elections elections = new Elections(electorsByDistrict, true); elections.addCandidate("Michel"); elections.addCandidate("Jerry"); elections.addCandidate("Johnny"); From dd86c3040763adcc0880a8b02ad4072e4b2cd564 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sat, 28 May 2022 09:18:34 +0500 Subject: [PATCH 02/23] duplicated-code: incrementing candidate votes counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Две строки кода, добавляющие голос за конкретного кандидата путем увеличения счетчика в соответствующем элементе массива голосов (по индексу кандидата). В целом это решение уже выглядит совсем не ООП, потому что использует примитивы для подсчета голосов и простые массивы для хранения счетчиков. Вместо этого можно использовать объекты Vote и специализированные объекты-коллекции для хранения счетчиков. Мне совершенно непонятно, каким образом можно перейти к ООП-решению из этого конкретного примера. Поэтому я просто выделю метод, содержащий эти две строки кода. Сначала выделю временные переменные - они станут аргументами нового метода, затем применю стандартный рефакторинг Extract Method. IDE сама предложит мне заменить второй случай дублирования кода. --- java/src/main/java/org/elections/Elections.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 3d6242f..ebaafb4 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -33,8 +33,7 @@ public void addCandidate(String candidate) { public void voteFor(String elector, String candidate, String electorDistrict) { if (!withDistrict) { if (candidates.contains(candidate)) { - int index = candidates.indexOf(candidate); - votesWithoutDistricts.set(index, votesWithoutDistricts.get(index) + 1); + incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); } else { candidates.add(candidate); votesWithoutDistricts.add(1); @@ -43,8 +42,7 @@ public void voteFor(String elector, String candidate, String electorDistrict) { if (votesWithDistricts.containsKey(electorDistrict)) { ArrayList districtVotes = votesWithDistricts.get(electorDistrict); if (candidates.contains(candidate)) { - int index = candidates.indexOf(candidate); - districtVotes.set(index, districtVotes.get(index) + 1); + incrementCandidateVotesCounter(candidate, candidates, districtVotes); } else { candidates.add(candidate); votesWithDistricts.forEach((district, votes) -> { @@ -56,6 +54,11 @@ public void voteFor(String elector, String candidate, String electorDistrict) { } } + private void incrementCandidateVotesCounter(String candidate, List candidates, ArrayList votesByCandidate) { + int index = candidates.indexOf(candidate); + votesByCandidate.set(index, votesByCandidate.get(index) + 1); + } + public Map results() { Map results = new HashMap<>(); Integer nbVotes = 0; From 57a1cf7389827d3707ab0fc3dbbb28dd7a7b98d2 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sun, 29 May 2022 09:17:48 +0500 Subject: [PATCH 03/23] obscured-intent: make adding new votes counter use 0 as default value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Код в методе voteFor в ветках, обрабатывающих случай отсутствия кандидата в списках, содержит Magic Literals и значительно отличается в части добавления информации о новом голосе. Однако по сути это одна и та же операция, состоящая из двух частей: добавлении нового счетчика для голосов кандидата и увеличение этого счетчика на единицу. Намек на это дает разный способ добавления нового элемента в массив счетчиков голосов для случаев с учетом и без учета округов. Без учета округов (т.е. при наличии только одного массива счетчиков) новый счетчик добавляется сразу с единицей в качестве начального значения. С учетом округов сначала в массивы счетчиков для каждого округа добавляются новые счетчики с начальным значением 0, а затем нужный (соответствующий текущему округу) счетчик увеличивается на единицу. Чтобы сделать две ветки кода максимально похожими, нужно разделить операцию добавления счетчика в ветке без учета округов на две: добавление с нулем в качестве начального значения и увеличение счетчика (отдельный метод, выделенный в предыдущем рефакторинге). --- java/src/main/java/org/elections/Elections.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index ebaafb4..5ade23f 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -36,7 +36,8 @@ public void voteFor(String elector, String candidate, String electorDistrict) { incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); } else { candidates.add(candidate); - votesWithoutDistricts.add(1); + votesWithoutDistricts.add(0); + incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); } } else { if (votesWithDistricts.containsKey(electorDistrict)) { @@ -45,10 +46,8 @@ public void voteFor(String elector, String candidate, String electorDistrict) { incrementCandidateVotesCounter(candidate, candidates, districtVotes); } else { candidates.add(candidate); - votesWithDistricts.forEach((district, votes) -> { - votes.add(0); - }); - districtVotes.set(candidates.size() - 1, districtVotes.get(candidates.size() - 1) + 1); + votesWithDistricts.forEach((district, votes) -> votes.add(0)); + incrementCandidateVotesCounter(candidate, candidates, districtVotes); } } } From a4d879c610da2cb6bf51a50c0adc167e4b7893ca Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sun, 29 May 2022 09:29:07 +0500 Subject: [PATCH 04/23] duplicated-code: increment votes counter unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit В методе voteFor ветки кода, обрабатывающие голос за ранее добавленного кандидата, содержат только одну инструкцию - увеличение счетчика. Эта же инструкция содержится и в ветках, обрабатывающих добавление нового кандидата. Инструкцию можно вынести за рамки условного оператора и удалить пустую ветку. --- java/src/main/java/org/elections/Elections.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 5ade23f..ba5ca52 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -32,23 +32,19 @@ public void addCandidate(String candidate) { public void voteFor(String elector, String candidate, String electorDistrict) { if (!withDistrict) { - if (candidates.contains(candidate)) { - incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); - } else { + if (!candidates.contains(candidate)) { candidates.add(candidate); votesWithoutDistricts.add(0); - incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); } + incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); } else { if (votesWithDistricts.containsKey(electorDistrict)) { ArrayList districtVotes = votesWithDistricts.get(electorDistrict); - if (candidates.contains(candidate)) { - incrementCandidateVotesCounter(candidate, candidates, districtVotes); - } else { + if (!candidates.contains(candidate)) { candidates.add(candidate); votesWithDistricts.forEach((district, votes) -> votes.add(0)); - incrementCandidateVotesCounter(candidate, candidates, districtVotes); } + incrementCandidateVotesCounter(candidate, candidates, districtVotes); } } } From 3ec9db51fed7039ad7dbcc1a231d4b4b638f9015 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sun, 29 May 2022 09:45:30 +0500 Subject: [PATCH 05/23] duplicated-code: generalize collection type for votes w/o district MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Операция добавления счетчика для нового кандидата для случаев с учетом и без учета округов выглядит практически одинаково, разница лишь в том, в какой тип коллекции нужно внести изменения. Можно обобщить случай без учета округов и представить массив счетчиков в виде словаря с единственной записью. Ключ этой записи - фиктивное название округа, а значение - массив счетчиков. Точно такая же коллекция используется для хранения счетчиков с учетом округов. Для этого я сначала добавил новое поле для хранения словаря счетчиков без учета округов. Затем продублировал все операции увеличения счетчиков, добавив к ним инструкцию добавления в новый словарь. В методе определения результата голосования ввел временную переменную для хранения массива счетчиков без учета округов, чтобы остальной код метода продолжал работать с массивом. В конце заменил значение этой переменной на значение из единственной записи нового словаря, а старое поле класса с массивом счетчиков без учета округов удалил. --- .../main/java/org/elections/Elections.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index ba5ca52..cf5caf2 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -4,9 +4,10 @@ import java.util.*; public class Elections { + private static final String NO_DISTRICT = "No district"; List candidates = new ArrayList<>(); List officialCandidates = new ArrayList<>(); - ArrayList votesWithoutDistricts = new ArrayList<>(); + Map> votesWithoutDistrict; Map> votesWithDistricts; private Map> list; private boolean withDistrict; @@ -15,6 +16,9 @@ public Elections(Map> list, boolean withDistrict) { this.list = list; this.withDistrict = withDistrict; + votesWithoutDistrict = new HashMap<>(); + votesWithoutDistrict.put(NO_DISTRICT, new ArrayList<>()); + votesWithDistricts = new HashMap<>(); votesWithDistricts.put("District 1", new ArrayList<>()); votesWithDistricts.put("District 2", new ArrayList<>()); @@ -24,7 +28,9 @@ public Elections(Map> list, boolean withDistrict) { public void addCandidate(String candidate) { officialCandidates.add(candidate); candidates.add(candidate); - votesWithoutDistricts.add(0); + + votesWithoutDistrict.get(NO_DISTRICT).add(0); + votesWithDistricts.get("District 1").add(0); votesWithDistricts.get("District 2").add(0); votesWithDistricts.get("District 3").add(0); @@ -34,17 +40,16 @@ public void voteFor(String elector, String candidate, String electorDistrict) { if (!withDistrict) { if (!candidates.contains(candidate)) { candidates.add(candidate); - votesWithoutDistricts.add(0); + votesWithoutDistrict.forEach((district, votes) -> votes.add(0)); } - incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistricts); + incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistrict.get(NO_DISTRICT)); } else { if (votesWithDistricts.containsKey(electorDistrict)) { - ArrayList districtVotes = votesWithDistricts.get(electorDistrict); if (!candidates.contains(candidate)) { candidates.add(candidate); votesWithDistricts.forEach((district, votes) -> votes.add(0)); } - incrementCandidateVotesCounter(candidate, candidates, districtVotes); + incrementCandidateVotesCounter(candidate, candidates, votesWithDistricts.get(electorDistrict)); } } } @@ -62,22 +67,23 @@ public Map results() { int nbValidVotes = 0; if (!withDistrict) { - nbVotes = votesWithoutDistricts.stream().reduce(0, Integer::sum); + final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); + nbVotes = votesForNoDistrict.stream().reduce(0, Integer::sum); for (int i = 0; i < officialCandidates.size(); i++) { int index = candidates.indexOf(officialCandidates.get(i)); - nbValidVotes += votesWithoutDistricts.get(index); + nbValidVotes += votesForNoDistrict.get(index); } - for (int i = 0; i < votesWithoutDistricts.size(); i++) { - Float candidatResult = ((float)votesWithoutDistricts.get(i) * 100) / nbValidVotes; + for (int i = 0; i < votesForNoDistrict.size(); i++) { + Float candidatResult = ((float) votesForNoDistrict.get(i) * 100) / nbValidVotes; String candidate = candidates.get(i); if (officialCandidates.contains(candidate)) { results.put(candidate, String.format(Locale.FRENCH, "%.2f%%", candidatResult)); } else { if (candidates.get(i).isEmpty()) { - blankVotes += votesWithoutDistricts.get(i); + blankVotes += votesForNoDistrict.get(i); } else { - nullVotes += votesWithoutDistricts.get(i); + nullVotes += votesForNoDistrict.get(i); } } } From 57997a75f32de0265258cf2e1a17261da0fdef81 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sun, 29 May 2022 10:13:41 +0500 Subject: [PATCH 06/23] duplicated-code: extract method for adding an unofficial candidate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Содержимое веток, обрабатывающих добавление нового кандидата при регистрации голосов, практически одинаково для случаев с учетом и без учета округов, поэтому можно выделить его в небольшой метод. Попутно выяснилось, что с точки зрения бизнес-логики такой кандидат (добавленный в ходе регистрации голосов) не является официально зарегистрированным и не входит в массив официальных кандидатов. Поэтому уже существующий метод создания кандидата до регистрации голосов был переименован в `addOfficialCandidate`. --- java/src/main/java/org/elections/Elections.java | 13 ++++++++----- java/src/test/java/org/elections/ElectionsTest.java | 12 ++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index cf5caf2..9875c94 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -25,7 +25,7 @@ public Elections(Map> list, boolean withDistrict) { votesWithDistricts.put("District 3", new ArrayList<>()); } - public void addCandidate(String candidate) { + public void addOfficialCandidate(String candidate) { officialCandidates.add(candidate); candidates.add(candidate); @@ -39,21 +39,24 @@ public void addCandidate(String candidate) { public void voteFor(String elector, String candidate, String electorDistrict) { if (!withDistrict) { if (!candidates.contains(candidate)) { - candidates.add(candidate); - votesWithoutDistrict.forEach((district, votes) -> votes.add(0)); + addUnofficialCandidate(candidate, votesWithoutDistrict); } incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistrict.get(NO_DISTRICT)); } else { if (votesWithDistricts.containsKey(electorDistrict)) { if (!candidates.contains(candidate)) { - candidates.add(candidate); - votesWithDistricts.forEach((district, votes) -> votes.add(0)); + addUnofficialCandidate(candidate, votesWithDistricts); } incrementCandidateVotesCounter(candidate, candidates, votesWithDistricts.get(electorDistrict)); } } } + private void addUnofficialCandidate(String candidate, Map> votesMap) { + candidates.add(candidate); + votesMap.forEach((district, votes) -> votes.add(0)); + } + private void incrementCandidateVotesCounter(String candidate, List candidates, ArrayList votesByCandidate) { int index = candidates.indexOf(candidate); votesByCandidate.set(index, votesByCandidate.get(index) + 1); diff --git a/java/src/test/java/org/elections/ElectionsTest.java b/java/src/test/java/org/elections/ElectionsTest.java index 5dce6b1..a650469 100644 --- a/java/src/test/java/org/elections/ElectionsTest.java +++ b/java/src/test/java/org/elections/ElectionsTest.java @@ -95,9 +95,9 @@ void tooManyVotes_shouldMakeAbstentionNegative() { @DisplayName("Approval-Test для подсчета голосов") void electionWithoutDistricts() { Elections elections = new Elections(electorsByDistrict, false); - elections.addCandidate("Michel"); - elections.addCandidate("Jerry"); - elections.addCandidate("Johnny"); + elections.addOfficialCandidate("Michel"); + elections.addOfficialCandidate("Jerry"); + elections.addOfficialCandidate("Johnny"); elections.voteFor("Bob", "Jerry", "District 1"); elections.voteFor("Jerry", "Jerry", "District 2"); @@ -136,9 +136,9 @@ void givenNoCandidatesWithDistricts_oneVoteForAnyCandidate_shouldThrow() { @Test void electionWithDistricts() { Elections elections = new Elections(electorsByDistrict, true); - elections.addCandidate("Michel"); - elections.addCandidate("Jerry"); - elections.addCandidate("Johnny"); + elections.addOfficialCandidate("Michel"); + elections.addOfficialCandidate("Jerry"); + elections.addOfficialCandidate("Johnny"); elections.voteFor("Bob", "Jerry", "District 1"); elections.voteFor("Jerry", "Jerry", "District 2"); From cbd76a25b14ccb0d5990e31b78f3d13f93800036 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Sun, 29 May 2022 10:27:10 +0500 Subject: [PATCH 07/23] duplicated-code: extract method for vote count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ветки метода `voteFor` теперь выглядят идентично, разница только в принимаемых аргументах. Можно легко применить "Выделение метода". В целом, все рефакторинги, начиная с коммита d3a78ef2, оказались частью более крупного рефакторинга Decompose Conditional. Код по-прежнему очень процедурный, в ходе рефакторингов не появились новые объекты. С точки зрения разделения кода на Данные, Вычисления и Действия тоже ничего не поменялось - код все так же использует поля класса, которые в данном случае практически являются глобальными переменными. --- .../main/java/org/elections/Elections.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 9875c94..4635b44 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -38,18 +38,17 @@ public void addOfficialCandidate(String candidate) { public void voteFor(String elector, String candidate, String electorDistrict) { if (!withDistrict) { - if (!candidates.contains(candidate)) { - addUnofficialCandidate(candidate, votesWithoutDistrict); - } - incrementCandidateVotesCounter(candidate, candidates, votesWithoutDistrict.get(NO_DISTRICT)); - } else { - if (votesWithDistricts.containsKey(electorDistrict)) { - if (!candidates.contains(candidate)) { - addUnofficialCandidate(candidate, votesWithDistricts); - } - incrementCandidateVotesCounter(candidate, candidates, votesWithDistricts.get(electorDistrict)); - } + countVote(candidate, votesWithoutDistrict, NO_DISTRICT); + } else if (votesWithDistricts.containsKey(electorDistrict)) { + countVote(candidate, votesWithDistricts, electorDistrict); + } + } + + private void countVote(String candidate, Map> votesMap, String districtName) { + if (!candidates.contains(candidate)) { + addUnofficialCandidate(candidate, votesMap); } + incrementCandidateVotesCounter(candidate, candidates, votesMap.get(districtName)); } private void addUnofficialCandidate(String candidate, Map> votesMap) { From b397b0f3e60bb3085b28cfb4390fa9187be6d4e4 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Mon, 30 May 2022 08:04:09 +0500 Subject: [PATCH 08/23] duplicated-code: extract method for counting total votes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Тип коллекции, используемой в подсчете общего количества голосов в случае без учета округов, можно обобщить до словаря (как в одном из предыдущих рефакторингов). После этого функция подсчета будет идентичной в обоих случаях, так что ее можно вынести в отдельный метод. Кроме того, собственно расчет общего количества голосов можно перенести ближе к месту его непосредственного использования при расчете итоговых статистик и форматировании результатов. --- .../src/main/java/org/elections/Elections.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 4635b44..a5fa25d 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -63,14 +63,13 @@ private void incrementCandidateVotesCounter(String candidate, List candi public Map results() { Map results = new HashMap<>(); - Integer nbVotes = 0; Integer nullVotes = 0; Integer blankVotes = 0; int nbValidVotes = 0; if (!withDistrict) { final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); - nbVotes = votesForNoDistrict.stream().reduce(0, Integer::sum); + for (int i = 0; i < officialCandidates.size(); i++) { int index = candidates.indexOf(officialCandidates.get(i)); nbValidVotes += votesForNoDistrict.get(index); @@ -90,11 +89,6 @@ public Map results() { } } } else { - for (Map.Entry> entry : votesWithDistricts.entrySet()) { - ArrayList districtVotes = entry.getValue(); - nbVotes += districtVotes.stream().reduce(0, Integer::sum); - } - for (int i = 0; i < officialCandidates.size(); i++) { int index = candidates.indexOf(officialCandidates.get(i)); for (Map.Entry> entry : votesWithDistricts.entrySet()) { @@ -138,6 +132,8 @@ public Map results() { } } + int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); + float blankResult = ((float)blankVotes * 100) / nbVotes; results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankResult)); @@ -152,4 +148,12 @@ public Map results() { return results; } + + private Integer countTotalVotes(Map> entries) { + int totalVotes = 0; + for (Map.Entry> entry : entries.entrySet()) { + totalVotes += entry.getValue().stream().reduce(0, Integer::sum); + } + return totalVotes; + } } From 6468f3f3aba1b8e233f2eb6512e475ff8827ddb6 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Mon, 30 May 2022 08:23:22 +0500 Subject: [PATCH 09/23] duplicated-code: extract method for counting official candidates votes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Тип коллекции, используемой в подсчете количества голосов за официальных кандидатов в случае без учета округов, можно обобщить до словаря (как в одном из предыдущих рефакторингов). После этого функция подсчета будет идентичной в обоих случаях, так что ее можно вынести в отдельный метод. --- .../main/java/org/elections/Elections.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index a5fa25d..04fc998 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -65,16 +65,10 @@ public Map results() { Map results = new HashMap<>(); Integer nullVotes = 0; Integer blankVotes = 0; - int nbValidVotes = 0; + int nbValidVotes = countOfficialCandidateVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); if (!withDistrict) { final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); - - for (int i = 0; i < officialCandidates.size(); i++) { - int index = candidates.indexOf(officialCandidates.get(i)); - nbValidVotes += votesForNoDistrict.get(index); - } - for (int i = 0; i < votesForNoDistrict.size(); i++) { Float candidatResult = ((float) votesForNoDistrict.get(i) * 100) / nbValidVotes; String candidate = candidates.get(i); @@ -89,14 +83,6 @@ public Map results() { } } } else { - for (int i = 0; i < officialCandidates.size(); i++) { - int index = candidates.indexOf(officialCandidates.get(i)); - for (Map.Entry> entry : votesWithDistricts.entrySet()) { - ArrayList districtVotes = entry.getValue(); - nbValidVotes += districtVotes.get(index); - } - } - Map officialCandidatesResult = new HashMap<>(); for (int i = 0; i < officialCandidates.size(); i++) { officialCandidatesResult.put(candidates.get(i), 0); @@ -149,6 +135,17 @@ public Map results() { return results; } + private int countOfficialCandidateVotes(Map> entries) { + int nbValidVotes = 0; + for (int i = 0; i < officialCandidates.size(); i++) { + for (Map.Entry> entry : entries.entrySet()) { + ArrayList districtVotes = entry.getValue(); + nbValidVotes += districtVotes.get(i); + } + } + return nbValidVotes; + } + private Integer countTotalVotes(Map> entries) { int totalVotes = 0; for (Map.Entry> entry : entries.entrySet()) { From 88aa61afe2a3b25a8474129ff397ad54a8a5cbee Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Mon, 30 May 2022 09:20:26 +0500 Subject: [PATCH 10/23] primitive-obsession: use Fraction to represent resulting percents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Это подготовительный рефакторинг, мотивированный тем, что большая часть процедуры расчета результатов - это классификация голосов на голоса за кандидата (с учетом округа или без), пустые бланки (empty) и ошибочные бланки (null). Однако выделить классификацию в отдельный метод с наскоку не получилось из-за того, что число пустых и ошибочных бланков хранится в примитиве Integer, а значит, должно быть возвращаемым из метода значением. Поскольку в методе будут рассчитываться три категории голосов, то и возвращаемых значений должно быть три. Поскольку в Java метод может возвращать только одно значение, нужно остальные два аккумулировать в каком-то объекте, передаваемом в метод по ссылке. Таким объектом вполне может стать объект Fraction, в чьи обязанности будет входить расчет процентного значения для каждой из категорий голосов. --- .../main/java/org/elections/Elections.java | 29 ++++++++----------- .../src/main/java/org/elections/Fraction.java | 29 +++++++++++++++++++ 2 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 java/src/main/java/org/elections/Fraction.java diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 04fc998..79a1467 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -1,6 +1,5 @@ package org.elections; -import java.text.DecimalFormat; import java.util.*; public class Elections { @@ -70,10 +69,10 @@ public Map results() { if (!withDistrict) { final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); for (int i = 0; i < votesForNoDistrict.size(); i++) { - Float candidatResult = ((float) votesForNoDistrict.get(i) * 100) / nbValidVotes; String candidate = candidates.get(i); if (officialCandidates.contains(candidate)) { - results.put(candidate, String.format(Locale.FRENCH, "%.2f%%", candidatResult)); + Fraction candidateResult = Fraction.withNumeratorDenominator(votesForNoDistrict.get(i), nbValidVotes); + results.put(candidate, String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); } else { if (candidates.get(i).isEmpty()) { blankVotes += votesForNoDistrict.get(i); @@ -91,12 +90,10 @@ public Map results() { ArrayList districtResult = new ArrayList<>(); ArrayList districtVotes = entry.getValue(); for (int i = 0; i < districtVotes.size(); i++) { - float candidateResult = 0; - if (nbValidVotes != 0) - candidateResult = ((float)districtVotes.get(i) * 100) / nbValidVotes; + Fraction candidateResult = Fraction.withNumeratorDenominator(districtVotes.get(i), nbValidVotes); String candidate = candidates.get(i); if (officialCandidates.contains(candidate)) { - districtResult.add(candidateResult); + districtResult.add(candidateResult.asPercent()); } else { if (candidates.get(i).isEmpty()) { blankVotes += districtVotes.get(i); @@ -113,24 +110,22 @@ public Map results() { officialCandidatesResult.put(candidates.get(districtWinnerIndex), officialCandidatesResult.get(candidates.get(districtWinnerIndex)) + 1); } for (int i = 0; i < officialCandidatesResult.size(); i++) { - Float ratioCandidate = ((float) officialCandidatesResult.get(candidates.get(i))) / officialCandidatesResult.size() * 100; - results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", ratioCandidate)); + Fraction ratio = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), officialCandidatesResult.size()); + results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", ratio.asPercent())); } } int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); - float blankResult = ((float)blankVotes * 100) / nbVotes; - results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankResult)); + Fraction blankResult = Fraction.withNumeratorDenominator(blankVotes, nbVotes); + results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankResult.asPercent())); - float nullResult = ((float)nullVotes * 100) / nbVotes; - results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullResult)); + Fraction nullResult = Fraction.withNumeratorDenominator(nullVotes, nbVotes); + results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullResult.asPercent())); int nbElectors = list.values().stream().map(List::size).reduce(0, Integer::sum); - DecimalFormat df = new DecimalFormat(); - df.setMaximumFractionDigits(2); - float abstentionResult = 100 - ((float) nbVotes * 100 / nbElectors); - results.put("Abstention", String.format(Locale.FRENCH, "%.2f%%", abstentionResult)); + Fraction abstentionResult = Fraction.withNumeratorDenominator(nbElectors - nbVotes, nbElectors); + results.put("Abstention", String.format(Locale.FRENCH, "%.2f%%", abstentionResult.asPercent())); return results; } diff --git a/java/src/main/java/org/elections/Fraction.java b/java/src/main/java/org/elections/Fraction.java new file mode 100644 index 0000000..aea6aa8 --- /dev/null +++ b/java/src/main/java/org/elections/Fraction.java @@ -0,0 +1,29 @@ +package org.elections; + +public class Fraction { + private int numerator; + private int denominator; + + public static Fraction withDenominator_(int denominator) { + return new Fraction().setNumerator_denominator_(0, denominator); + } + + public static Fraction withNumeratorDenominator(int numerator, int denominator) { + return new Fraction().setNumerator_denominator_(numerator, denominator); + } + + private Fraction() {} + + private Fraction setNumerator_denominator_(int numerator, int denominator) { + this.numerator = numerator; + this.denominator = denominator; + return this; + } + + public float asPercent() { + if (this.denominator == 0) { + return 0; + } + return (float)this.numerator * 100 / this.denominator; + } +} From faa69362036a05e91921a8f93ca790d2ade89a9c Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Mon, 30 May 2022 09:26:22 +0500 Subject: [PATCH 11/23] primitive-obsession: use Fraction to represent blank and null votes --- .../main/java/org/elections/Elections.java | 22 ++++++++----------- .../src/main/java/org/elections/Fraction.java | 6 ++++- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 79a1467..b295559 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -62,9 +62,10 @@ private void incrementCandidateVotesCounter(String candidate, List candi public Map results() { Map results = new HashMap<>(); - Integer nullVotes = 0; - Integer blankVotes = 0; int nbValidVotes = countOfficialCandidateVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); + int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); + Fraction nullVotes = Fraction.withDenominator(nbVotes); + Fraction blankVotes = Fraction.withDenominator(nbVotes); if (!withDistrict) { final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); @@ -75,9 +76,9 @@ public Map results() { results.put(candidate, String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); } else { if (candidates.get(i).isEmpty()) { - blankVotes += votesForNoDistrict.get(i); + blankVotes.addToNumerator(votesForNoDistrict.get(i)); } else { - nullVotes += votesForNoDistrict.get(i); + nullVotes.addToNumerator(votesForNoDistrict.get(i)); } } } @@ -96,9 +97,9 @@ public Map results() { districtResult.add(candidateResult.asPercent()); } else { if (candidates.get(i).isEmpty()) { - blankVotes += districtVotes.get(i); + blankVotes.addToNumerator(districtVotes.get(i)); } else { - nullVotes += districtVotes.get(i); + nullVotes.addToNumerator(districtVotes.get(i)); } } } @@ -115,13 +116,8 @@ public Map results() { } } - int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); - - Fraction blankResult = Fraction.withNumeratorDenominator(blankVotes, nbVotes); - results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankResult.asPercent())); - - Fraction nullResult = Fraction.withNumeratorDenominator(nullVotes, nbVotes); - results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullResult.asPercent())); + results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankVotes.asPercent())); + results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullVotes.asPercent())); int nbElectors = list.values().stream().map(List::size).reduce(0, Integer::sum); Fraction abstentionResult = Fraction.withNumeratorDenominator(nbElectors - nbVotes, nbElectors); diff --git a/java/src/main/java/org/elections/Fraction.java b/java/src/main/java/org/elections/Fraction.java index aea6aa8..a988c6c 100644 --- a/java/src/main/java/org/elections/Fraction.java +++ b/java/src/main/java/org/elections/Fraction.java @@ -4,7 +4,7 @@ public class Fraction { private int numerator; private int denominator; - public static Fraction withDenominator_(int denominator) { + public static Fraction withDenominator(int denominator) { return new Fraction().setNumerator_denominator_(0, denominator); } @@ -26,4 +26,8 @@ public float asPercent() { } return (float)this.numerator * 100 / this.denominator; } + + public void addToNumerator(Integer amount) { + this.numerator += amount; + } } From 90740be59447f7499682aa5b39394c5cfc2d647c Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Mon, 30 May 2022 09:45:54 +0500 Subject: [PATCH 12/23] duplicated-code: extract votes categorizing method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit В итоге после использования аккумулирующих объектов Fraction для подсчета пустых и ошибочных бланков функция категоризации бланков стала практически идентичной для случаев с учетом и без учета округов. Поэтому можно выделить ее в новый метод fillVoteBuckets. --- .../main/java/org/elections/Elections.java | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index b295559..3678304 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -67,42 +67,26 @@ public Map results() { Fraction nullVotes = Fraction.withDenominator(nbVotes); Fraction blankVotes = Fraction.withDenominator(nbVotes); + Map officialCandidatesResult = new HashMap<>(); + for (int i = 0; i < officialCandidates.size(); i++) { + officialCandidatesResult.put(candidates.get(i), 0); + } if (!withDistrict) { - final ArrayList votesForNoDistrict = votesWithoutDistrict.get(NO_DISTRICT); - for (int i = 0; i < votesForNoDistrict.size(); i++) { - String candidate = candidates.get(i); - if (officialCandidates.contains(candidate)) { - Fraction candidateResult = Fraction.withNumeratorDenominator(votesForNoDistrict.get(i), nbValidVotes); - results.put(candidate, String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); - } else { - if (candidates.get(i).isEmpty()) { - blankVotes.addToNumerator(votesForNoDistrict.get(i)); - } else { - nullVotes.addToNumerator(votesForNoDistrict.get(i)); - } + for (Map.Entry> entry : votesWithoutDistrict.entrySet()) { + ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); + + for (int i = 0; i < officialCandidates.size(); i++) { + officialCandidatesResult.put(officialCandidates.get(i), districtResult.get(i)); } } - } else { - Map officialCandidatesResult = new HashMap<>(); - for (int i = 0; i < officialCandidates.size(); i++) { - officialCandidatesResult.put(candidates.get(i), 0); + for (int i = 0; i < officialCandidatesResult.size(); i++) { + Fraction candidateResult = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), nbValidVotes); + results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); } + } else { for (Map.Entry> entry : votesWithDistricts.entrySet()) { - ArrayList districtResult = new ArrayList<>(); - ArrayList districtVotes = entry.getValue(); - for (int i = 0; i < districtVotes.size(); i++) { - Fraction candidateResult = Fraction.withNumeratorDenominator(districtVotes.get(i), nbValidVotes); - String candidate = candidates.get(i); - if (officialCandidates.contains(candidate)) { - districtResult.add(candidateResult.asPercent()); - } else { - if (candidates.get(i).isEmpty()) { - blankVotes.addToNumerator(districtVotes.get(i)); - } else { - nullVotes.addToNumerator(districtVotes.get(i)); - } - } - } + ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); + int districtWinnerIndex = 0; for (int i = 1; i < districtResult.size(); i++) { if (districtResult.get(districtWinnerIndex) < districtResult.get(i)) @@ -126,6 +110,24 @@ public Map results() { return results; } + private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVotes, Map.Entry> entry) { + ArrayList districtResult = new ArrayList<>(); + ArrayList districtVotes = entry.getValue(); + for (int i = 0; i < districtVotes.size(); i++) { + String candidate = candidates.get(i); + if (officialCandidates.contains(candidate)) { + districtResult.add(districtVotes.get(i)); + } else { + if (candidates.get(i).isEmpty()) { + blankVotes.addToNumerator(districtVotes.get(i)); + } else { + nullVotes.addToNumerator(districtVotes.get(i)); + } + } + } + return districtResult; + } + private int countOfficialCandidateVotes(Map> entries) { int nbValidVotes = 0; for (int i = 0; i < officialCandidates.size(); i++) { From 982573e7d32cb5cf58fc6a68211bcd4af759482b Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 08:04:18 +0500 Subject: [PATCH 13/23] primitive-obsession: store electors in a dedicated collection class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Я пока не знаю, куда двинуться дальше с рефакторингом метода подсчета результатов голосования. Код явно очень сильно зависит от структур данных, это проявление запаха "Primitive Obsession". Зависимостей от конкретной структуры данных очень много, проще всего было инкапсулировать информацию о списке избирателей в специальном классе-коллекции Electors. В дальнейшем это поможет реализовать требование о пометке голоса, в котором округ не соответствует округу избирателя, как ошибочного. --- .../main/java/org/elections/Elections.java | 6 ++-- java/src/main/java/org/elections/Elector.java | 16 ++++++++++ .../src/main/java/org/elections/Electors.java | 32 +++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 java/src/main/java/org/elections/Elector.java create mode 100644 java/src/main/java/org/elections/Electors.java diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 3678304..576e885 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -8,11 +8,11 @@ public class Elections { List officialCandidates = new ArrayList<>(); Map> votesWithoutDistrict; Map> votesWithDistricts; - private Map> list; + private final Electors electors; private boolean withDistrict; public Elections(Map> list, boolean withDistrict) { - this.list = list; + electors = Electors.fromMapByDistrict(list); this.withDistrict = withDistrict; votesWithoutDistrict = new HashMap<>(); @@ -103,7 +103,7 @@ public Map results() { results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankVotes.asPercent())); results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullVotes.asPercent())); - int nbElectors = list.values().stream().map(List::size).reduce(0, Integer::sum); + int nbElectors = electors.size(); Fraction abstentionResult = Fraction.withNumeratorDenominator(nbElectors - nbVotes, nbElectors); results.put("Abstention", String.format(Locale.FRENCH, "%.2f%%", abstentionResult.asPercent())); diff --git a/java/src/main/java/org/elections/Elector.java b/java/src/main/java/org/elections/Elector.java new file mode 100644 index 0000000..acfe7ea --- /dev/null +++ b/java/src/main/java/org/elections/Elector.java @@ -0,0 +1,16 @@ +package org.elections; + +public class Elector { + private String district; + private String name; + + public static Elector withDistrict_Name_(String district, String name) { + return new Elector().setDistrict_Name_(district, name); + } + + private Elector setDistrict_Name_(String district, String name) { + this.district = district; + this.name = name; + return this; + } +} diff --git a/java/src/main/java/org/elections/Electors.java b/java/src/main/java/org/elections/Electors.java new file mode 100644 index 0000000..6fe547c --- /dev/null +++ b/java/src/main/java/org/elections/Electors.java @@ -0,0 +1,32 @@ +package org.elections; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public class Electors { + private final List electors; + + private Electors() { + electors = new ArrayList<>(); + } + + public static Electors fromMapByDistrict(Map> electorsByDistrict) { + Electors instance = new Electors(); + for(Map.Entry> district : electorsByDistrict.entrySet()) { + instance.addAll(district.getValue().stream() + .map(el -> Elector.withDistrict_Name_(district.getKey(), el)) + .collect(Collectors.toList())); + } + return instance; + } + + private void addAll(List electorList) { + electors.addAll(electorList); + } + + public int size() { + return electors.size(); + } +} From 0198492f2ad59aaf0494d46bd8c2f00b2c5d260a Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 08:30:50 +0500 Subject: [PATCH 14/23] primitive-obsession: store candidates in a dedicated collection class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Следующий шаг - инкапсуляция списка кандидатов в выделенном классе. Полностью перейти на использование нового объекта не получилось, в этом коммите реализован отказ от использования отдельного списка официально зарегистрированных кандидатов. Вместо него все обращения идут к новому классу-коллекции Candidates. --- .../main/java/org/elections/Candidate.java | 27 ++++++++++++ .../main/java/org/elections/Candidates.java | 41 +++++++++++++++++++ .../main/java/org/elections/Elections.java | 23 +++++++---- 3 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 java/src/main/java/org/elections/Candidate.java create mode 100644 java/src/main/java/org/elections/Candidates.java diff --git a/java/src/main/java/org/elections/Candidate.java b/java/src/main/java/org/elections/Candidate.java new file mode 100644 index 0000000..40fefa4 --- /dev/null +++ b/java/src/main/java/org/elections/Candidate.java @@ -0,0 +1,27 @@ +package org.elections; + +public class Candidate { + private final String name; + private final boolean isOfficial; + + public Candidate(String name, boolean isOfficial) { + this.name = name; + this.isOfficial = isOfficial; + } + + public static Candidate official(String name) { + return new Candidate(name, true); + } + + public static Candidate unofficial(String name) { + return new Candidate(name, false); + } + + public String name() { + return this.name; + } + + public boolean isOfficial() { + return this.isOfficial; + } +} diff --git a/java/src/main/java/org/elections/Candidates.java b/java/src/main/java/org/elections/Candidates.java new file mode 100644 index 0000000..4ba6e6c --- /dev/null +++ b/java/src/main/java/org/elections/Candidates.java @@ -0,0 +1,41 @@ +package org.elections; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +public class Candidates { + private final List candidates = new ArrayList<>(); + + public void addOfficial(String name) { + candidates.add(Candidate.official(name)); + } + + public void addUnofficial(String name) { + candidates.add(Candidate.unofficial(name)); + } + + public int indexOf(String candidate) { + for (int i = 0; i < candidates.size(); i++) { + if (candidate.equals(candidates.get(i).name())) { + return i; + } + } + return -1; + } + + public List officialCandidates() { + return candidates.stream() + .filter(Candidate::isOfficial) + .map(Candidate::name) + .collect(Collectors.toUnmodifiableList()); + } + + public long officialCandidatesCount() { + return candidates.stream().filter(Candidate::isOfficial).count(); + } + + public boolean isOfficial(String candidate) { + return candidates.stream().filter(c -> candidate.equals(c.name())).anyMatch(Candidate::isOfficial); + } +} diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 576e885..743433f 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -5,14 +5,16 @@ public class Elections { private static final String NO_DISTRICT = "No district"; List candidates = new ArrayList<>(); - List officialCandidates = new ArrayList<>(); Map> votesWithoutDistrict; Map> votesWithDistricts; private final Electors electors; + private final Candidates candidatesEntity; private boolean withDistrict; public Elections(Map> list, boolean withDistrict) { electors = Electors.fromMapByDistrict(list); + candidatesEntity = new Candidates(); + this.withDistrict = withDistrict; votesWithoutDistrict = new HashMap<>(); @@ -25,8 +27,8 @@ public Elections(Map> list, boolean withDistrict) { } public void addOfficialCandidate(String candidate) { - officialCandidates.add(candidate); candidates.add(candidate); + candidatesEntity.addOfficial(candidate); votesWithoutDistrict.get(NO_DISTRICT).add(0); @@ -47,15 +49,16 @@ private void countVote(String candidate, Map> votesMa if (!candidates.contains(candidate)) { addUnofficialCandidate(candidate, votesMap); } - incrementCandidateVotesCounter(candidate, candidates, votesMap.get(districtName)); + incrementCandidateVotesCounter(candidate, candidatesEntity, votesMap.get(districtName)); } private void addUnofficialCandidate(String candidate, Map> votesMap) { candidates.add(candidate); + candidatesEntity.addUnofficial(candidate); votesMap.forEach((district, votes) -> votes.add(0)); } - private void incrementCandidateVotesCounter(String candidate, List candidates, ArrayList votesByCandidate) { + private void incrementCandidateVotesCounter(String candidate, Candidates candidates, ArrayList votesByCandidate) { int index = candidates.indexOf(candidate); votesByCandidate.set(index, votesByCandidate.get(index) + 1); } @@ -67,16 +70,18 @@ public Map results() { Fraction nullVotes = Fraction.withDenominator(nbVotes); Fraction blankVotes = Fraction.withDenominator(nbVotes); + List offCand = candidatesEntity.officialCandidates(); + candidatesEntity.officialCandidatesCount(); Map officialCandidatesResult = new HashMap<>(); - for (int i = 0; i < officialCandidates.size(); i++) { + for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { officialCandidatesResult.put(candidates.get(i), 0); } if (!withDistrict) { for (Map.Entry> entry : votesWithoutDistrict.entrySet()) { ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); - for (int i = 0; i < officialCandidates.size(); i++) { - officialCandidatesResult.put(officialCandidates.get(i), districtResult.get(i)); + for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { + officialCandidatesResult.put(candidates.get(i), districtResult.get(i)); } } for (int i = 0; i < officialCandidatesResult.size(); i++) { @@ -115,7 +120,7 @@ private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVot ArrayList districtVotes = entry.getValue(); for (int i = 0; i < districtVotes.size(); i++) { String candidate = candidates.get(i); - if (officialCandidates.contains(candidate)) { + if (candidatesEntity.isOfficial(candidate)) { districtResult.add(districtVotes.get(i)); } else { if (candidates.get(i).isEmpty()) { @@ -130,7 +135,7 @@ private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVot private int countOfficialCandidateVotes(Map> entries) { int nbValidVotes = 0; - for (int i = 0; i < officialCandidates.size(); i++) { + for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { for (Map.Entry> entry : entries.entrySet()) { ArrayList districtVotes = entry.getValue(); nbValidVotes += districtVotes.get(i); From d5b6733d3c68441cb7f6891bf2dc7482f9fb4ee8 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 08:47:07 +0500 Subject: [PATCH 15/23] primitive-obsession: use dedicated Candidates collection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Второй этап замены простого списка кандидатов на специализированный объект-коллекцию Candidates. --- .../main/java/org/elections/Candidates.java | 15 +++++++++- .../main/java/org/elections/Elections.java | 30 ++++++++----------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/java/src/main/java/org/elections/Candidates.java b/java/src/main/java/org/elections/Candidates.java index 4ba6e6c..2e6a554 100644 --- a/java/src/main/java/org/elections/Candidates.java +++ b/java/src/main/java/org/elections/Candidates.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Predicate; import java.util.stream.Collectors; public class Candidates { @@ -36,6 +37,18 @@ public long officialCandidatesCount() { } public boolean isOfficial(String candidate) { - return candidates.stream().filter(c -> candidate.equals(c.name())).anyMatch(Candidate::isOfficial); + return candidates.stream().filter(sameNameAs(candidate)).anyMatch(Candidate::isOfficial); + } + + public boolean isUnregistered(String candidate) { + return candidates.stream().noneMatch(sameNameAs(candidate)); + } + + private Predicate sameNameAs(String candidate) { + return c -> candidate.equals(c.name()); + } + + public String get(int index) { + return candidates.get(index).name(); } } diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 743433f..686f7a9 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -4,16 +4,15 @@ public class Elections { private static final String NO_DISTRICT = "No district"; - List candidates = new ArrayList<>(); Map> votesWithoutDistrict; Map> votesWithDistricts; private final Electors electors; - private final Candidates candidatesEntity; + private final Candidates candidates; private boolean withDistrict; public Elections(Map> list, boolean withDistrict) { electors = Electors.fromMapByDistrict(list); - candidatesEntity = new Candidates(); + candidates = new Candidates(); this.withDistrict = withDistrict; @@ -27,8 +26,7 @@ public Elections(Map> list, boolean withDistrict) { } public void addOfficialCandidate(String candidate) { - candidates.add(candidate); - candidatesEntity.addOfficial(candidate); + candidates.addOfficial(candidate); votesWithoutDistrict.get(NO_DISTRICT).add(0); @@ -46,15 +44,14 @@ public void voteFor(String elector, String candidate, String electorDistrict) { } private void countVote(String candidate, Map> votesMap, String districtName) { - if (!candidates.contains(candidate)) { + if (candidates.isUnregistered(candidate)) { addUnofficialCandidate(candidate, votesMap); } - incrementCandidateVotesCounter(candidate, candidatesEntity, votesMap.get(districtName)); + incrementCandidateVotesCounter(candidate, candidates, votesMap.get(districtName)); } private void addUnofficialCandidate(String candidate, Map> votesMap) { - candidates.add(candidate); - candidatesEntity.addUnofficial(candidate); + candidates.addUnofficial(candidate); votesMap.forEach((district, votes) -> votes.add(0)); } @@ -70,17 +67,15 @@ public Map results() { Fraction nullVotes = Fraction.withDenominator(nbVotes); Fraction blankVotes = Fraction.withDenominator(nbVotes); - List offCand = candidatesEntity.officialCandidates(); - candidatesEntity.officialCandidatesCount(); Map officialCandidatesResult = new HashMap<>(); - for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { officialCandidatesResult.put(candidates.get(i), 0); } if (!withDistrict) { for (Map.Entry> entry : votesWithoutDistrict.entrySet()) { ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); - for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { officialCandidatesResult.put(candidates.get(i), districtResult.get(i)); } } @@ -97,7 +92,8 @@ public Map results() { if (districtResult.get(districtWinnerIndex) < districtResult.get(i)) districtWinnerIndex = i; } - officialCandidatesResult.put(candidates.get(districtWinnerIndex), officialCandidatesResult.get(candidates.get(districtWinnerIndex)) + 1); + final String districtWinner = candidates.get(districtWinnerIndex); + officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); } for (int i = 0; i < officialCandidatesResult.size(); i++) { Fraction ratio = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), officialCandidatesResult.size()); @@ -120,10 +116,10 @@ private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVot ArrayList districtVotes = entry.getValue(); for (int i = 0; i < districtVotes.size(); i++) { String candidate = candidates.get(i); - if (candidatesEntity.isOfficial(candidate)) { + if (candidates.isOfficial(candidate)) { districtResult.add(districtVotes.get(i)); } else { - if (candidates.get(i).isEmpty()) { + if (candidate.isEmpty()) { blankVotes.addToNumerator(districtVotes.get(i)); } else { nullVotes.addToNumerator(districtVotes.get(i)); @@ -135,7 +131,7 @@ private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVot private int countOfficialCandidateVotes(Map> entries) { int nbValidVotes = 0; - for (int i = 0; i < candidatesEntity.officialCandidatesCount(); i++) { + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { for (Map.Entry> entry : entries.entrySet()) { ArrayList districtVotes = entry.getValue(); nbValidVotes += districtVotes.get(i); From 2c5d18b986a9352cf896f2ee10288ed9ba9c038c Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 08:58:29 +0500 Subject: [PATCH 16/23] no-smell: move vote type definition to Vote class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Не знаю, к какой категории запахов и рефакторингов отнести эти изменения. Суть их заключается в том, что созданный класс Vote обладает всей информацией для того, чтобы определить свой тип (пустой или ошибочный в рамках данного коммита). Соответственно, подсчет количества таких голосов можно смело переместить из цикла, обрабатывающего голоса, в коллекцию Votes. В результате стало возможным отказаться от использования объектов Fraction в качестве "накопителей" при подсчете и сделать их по-настоящему неизменяемыми (с final полями). --- .../main/java/org/elections/Candidate.java | 8 +++++ .../main/java/org/elections/Candidates.java | 7 ++++ .../main/java/org/elections/Elections.java | 35 +++++++++---------- java/src/main/java/org/elections/Elector.java | 4 +++ .../src/main/java/org/elections/Electors.java | 8 +++++ .../src/main/java/org/elections/Fraction.java | 21 +++-------- java/src/main/java/org/elections/Vote.java | 25 +++++++++++++ java/src/main/java/org/elections/Votes.java | 29 +++++++++++++++ 8 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 java/src/main/java/org/elections/Vote.java create mode 100644 java/src/main/java/org/elections/Votes.java diff --git a/java/src/main/java/org/elections/Candidate.java b/java/src/main/java/org/elections/Candidate.java index 40fefa4..a3b4009 100644 --- a/java/src/main/java/org/elections/Candidate.java +++ b/java/src/main/java/org/elections/Candidate.java @@ -24,4 +24,12 @@ public String name() { public boolean isOfficial() { return this.isOfficial; } + + public boolean hasName(String candidateName) { + return this.name.equals(candidateName); + } + + public boolean isBlank() { + return this.name.trim().isEmpty(); + } } diff --git a/java/src/main/java/org/elections/Candidates.java b/java/src/main/java/org/elections/Candidates.java index 2e6a554..561737a 100644 --- a/java/src/main/java/org/elections/Candidates.java +++ b/java/src/main/java/org/elections/Candidates.java @@ -51,4 +51,11 @@ private Predicate sameNameAs(String candidate) { public String get(int index) { return candidates.get(index).name(); } + + public Candidate findByName(String candidateName) { + return candidates.stream() + .filter(candidate -> candidate.hasName(candidateName)) + .findFirst() + .orElse(null); + } } diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 686f7a9..269ac52 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -8,11 +8,13 @@ public class Elections { Map> votesWithDistricts; private final Electors electors; private final Candidates candidates; + private final Votes votes; private boolean withDistrict; public Elections(Map> list, boolean withDistrict) { electors = Electors.fromMapByDistrict(list); candidates = new Candidates(); + votes = new Votes(); this.withDistrict = withDistrict; @@ -35,12 +37,15 @@ public void addOfficialCandidate(String candidate) { votesWithDistricts.get("District 3").add(0); } - public void voteFor(String elector, String candidate, String electorDistrict) { + public void voteFor(String electorName, String candidateName, String electorDistrict) { if (!withDistrict) { - countVote(candidate, votesWithoutDistrict, NO_DISTRICT); + countVote(candidateName, votesWithoutDistrict, NO_DISTRICT); } else if (votesWithDistricts.containsKey(electorDistrict)) { - countVote(candidate, votesWithDistricts, electorDistrict); + countVote(candidateName, votesWithDistricts, electorDistrict); } + votes.registerVote(electorDistrict, + electors.findByName(electorName), + candidates.findByName(candidateName)); } private void countVote(String candidate, Map> votesMap, String districtName) { @@ -64,8 +69,6 @@ public Map results() { Map results = new HashMap<>(); int nbValidVotes = countOfficialCandidateVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); - Fraction nullVotes = Fraction.withDenominator(nbVotes); - Fraction blankVotes = Fraction.withDenominator(nbVotes); Map officialCandidatesResult = new HashMap<>(); for (int i = 0; i < candidates.officialCandidatesCount(); i++) { @@ -73,7 +76,7 @@ public Map results() { } if (!withDistrict) { for (Map.Entry> entry : votesWithoutDistrict.entrySet()) { - ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); + ArrayList districtResult = fillVoteBuckets(entry); for (int i = 0; i < candidates.officialCandidatesCount(); i++) { officialCandidatesResult.put(candidates.get(i), districtResult.get(i)); @@ -85,7 +88,7 @@ public Map results() { } } else { for (Map.Entry> entry : votesWithDistricts.entrySet()) { - ArrayList districtResult = fillVoteBuckets(nullVotes, blankVotes, entry); + ArrayList districtResult = fillVoteBuckets(entry); int districtWinnerIndex = 0; for (int i = 1; i < districtResult.size(); i++) { @@ -101,8 +104,11 @@ public Map results() { } } - results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blankVotes.asPercent())); - results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nullVotes.asPercent())); + Fraction blanks = Fraction.withNumeratorDenominator(votes.countBlanks(), nbVotes); + results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blanks.asPercent())); + + Fraction nulls = Fraction.withNumeratorDenominator(votes.countNulls(), nbVotes); + results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nulls.asPercent())); int nbElectors = electors.size(); Fraction abstentionResult = Fraction.withNumeratorDenominator(nbElectors - nbVotes, nbElectors); @@ -111,19 +117,12 @@ public Map results() { return results; } - private ArrayList fillVoteBuckets(Fraction nullVotes, Fraction blankVotes, Map.Entry> entry) { + private ArrayList fillVoteBuckets(Map.Entry> entry) { ArrayList districtResult = new ArrayList<>(); ArrayList districtVotes = entry.getValue(); for (int i = 0; i < districtVotes.size(); i++) { - String candidate = candidates.get(i); - if (candidates.isOfficial(candidate)) { + if (candidates.isOfficial(candidates.get(i))) { districtResult.add(districtVotes.get(i)); - } else { - if (candidate.isEmpty()) { - blankVotes.addToNumerator(districtVotes.get(i)); - } else { - nullVotes.addToNumerator(districtVotes.get(i)); - } } } return districtResult; diff --git a/java/src/main/java/org/elections/Elector.java b/java/src/main/java/org/elections/Elector.java index acfe7ea..962e54c 100644 --- a/java/src/main/java/org/elections/Elector.java +++ b/java/src/main/java/org/elections/Elector.java @@ -13,4 +13,8 @@ private Elector setDistrict_Name_(String district, String name) { this.name = name; return this; } + + public boolean hasName(String electorName) { + return this.name.equals(electorName); + } } diff --git a/java/src/main/java/org/elections/Electors.java b/java/src/main/java/org/elections/Electors.java index 6fe547c..402f70f 100644 --- a/java/src/main/java/org/elections/Electors.java +++ b/java/src/main/java/org/elections/Electors.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Collectors; public class Electors { @@ -29,4 +30,11 @@ private void addAll(List electorList) { public int size() { return electors.size(); } + + public Elector findByName(String electorName) { + return electors.stream() + .filter(elector -> elector.hasName(electorName)) + .findAny() + .orElse(null); + } } diff --git a/java/src/main/java/org/elections/Fraction.java b/java/src/main/java/org/elections/Fraction.java index a988c6c..855a6e8 100644 --- a/java/src/main/java/org/elections/Fraction.java +++ b/java/src/main/java/org/elections/Fraction.java @@ -1,23 +1,16 @@ package org.elections; public class Fraction { - private int numerator; - private int denominator; + private final long numerator; + private final long denominator; - public static Fraction withDenominator(int denominator) { - return new Fraction().setNumerator_denominator_(0, denominator); + public static Fraction withNumeratorDenominator(long numerator, int denominator) { + return new Fraction(numerator, denominator); } - public static Fraction withNumeratorDenominator(int numerator, int denominator) { - return new Fraction().setNumerator_denominator_(numerator, denominator); - } - - private Fraction() {} - - private Fraction setNumerator_denominator_(int numerator, int denominator) { + private Fraction(long numerator, long denominator) { this.numerator = numerator; this.denominator = denominator; - return this; } public float asPercent() { @@ -26,8 +19,4 @@ public float asPercent() { } return (float)this.numerator * 100 / this.denominator; } - - public void addToNumerator(Integer amount) { - this.numerator += amount; - } } diff --git a/java/src/main/java/org/elections/Vote.java b/java/src/main/java/org/elections/Vote.java new file mode 100644 index 0000000..f756a91 --- /dev/null +++ b/java/src/main/java/org/elections/Vote.java @@ -0,0 +1,25 @@ +package org.elections; + +public class Vote { + private final Elector elector; + private final Candidate candidate; + private final String district; + + public Vote(Elector elector, Candidate candidate, String district) { + this.elector = elector; + this.candidate = candidate; + this.district = district; + } + + public boolean isBlank() { + return this.candidate.isBlank(); + } + + public boolean hasCandidate() { + return !this.isBlank(); + } + + public boolean isForUnofficialCandidate() { + return !this.candidate.isOfficial(); + } +} diff --git a/java/src/main/java/org/elections/Votes.java b/java/src/main/java/org/elections/Votes.java new file mode 100644 index 0000000..e42aa72 --- /dev/null +++ b/java/src/main/java/org/elections/Votes.java @@ -0,0 +1,29 @@ +package org.elections; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +public class Votes { + private final List votes; + + public Votes() { + votes = new ArrayList<>(); + } + + public void registerVote(String district, Elector elector, Candidate candidate) { + votes.add(new Vote(elector, candidate, district)); + } + + public long countBlanks() { + return errorVotesStream().filter(Vote::isBlank).count(); + } + + public long countNulls() { + return errorVotesStream().filter(Vote::hasCandidate).count(); + } + + private Stream errorVotesStream() { + return votes.stream().filter(Vote::isForUnofficialCandidate); + } +} From 1e2d0f930d99d88c1b2163fd7a408f1dec91822d Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 09:41:09 +0500 Subject: [PATCH 17/23] no-smell: move all vote type counters to Votes collection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Все подсчеты количества голосов разных типов стало возможно перенести в класс-коллекцию Votes. В результате оказались не нужны многословные методы, подсчитывавшие голоса в циклах и с помощью полей класса (по сути, глобальных переменных) --- .../main/java/org/elections/Elections.java | 22 ++----------------- .../src/main/java/org/elections/Fraction.java | 2 +- java/src/main/java/org/elections/Votes.java | 8 +++++++ 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 269ac52..e666061 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -67,8 +67,8 @@ private void incrementCandidateVotesCounter(String candidate, Candidates candida public Map results() { Map results = new HashMap<>(); - int nbValidVotes = countOfficialCandidateVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); - int nbVotes = countTotalVotes(withDistrict ? votesWithDistricts:votesWithoutDistrict); + long nbValidVotes = votes.countValid(); + long nbVotes = votes.countAllVotes(); Map officialCandidatesResult = new HashMap<>(); for (int i = 0; i < candidates.officialCandidatesCount(); i++) { @@ -128,22 +128,4 @@ private ArrayList fillVoteBuckets(Map.Entry> return districtResult; } - private int countOfficialCandidateVotes(Map> entries) { - int nbValidVotes = 0; - for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - for (Map.Entry> entry : entries.entrySet()) { - ArrayList districtVotes = entry.getValue(); - nbValidVotes += districtVotes.get(i); - } - } - return nbValidVotes; - } - - private Integer countTotalVotes(Map> entries) { - int totalVotes = 0; - for (Map.Entry> entry : entries.entrySet()) { - totalVotes += entry.getValue().stream().reduce(0, Integer::sum); - } - return totalVotes; - } } diff --git a/java/src/main/java/org/elections/Fraction.java b/java/src/main/java/org/elections/Fraction.java index 855a6e8..1fb9710 100644 --- a/java/src/main/java/org/elections/Fraction.java +++ b/java/src/main/java/org/elections/Fraction.java @@ -4,7 +4,7 @@ public class Fraction { private final long numerator; private final long denominator; - public static Fraction withNumeratorDenominator(long numerator, int denominator) { + public static Fraction withNumeratorDenominator(long numerator, long denominator) { return new Fraction(numerator, denominator); } diff --git a/java/src/main/java/org/elections/Votes.java b/java/src/main/java/org/elections/Votes.java index e42aa72..d9863de 100644 --- a/java/src/main/java/org/elections/Votes.java +++ b/java/src/main/java/org/elections/Votes.java @@ -26,4 +26,12 @@ public long countNulls() { private Stream errorVotesStream() { return votes.stream().filter(Vote::isForUnofficialCandidate); } + + public long countValid() { + return votes.size() - errorVotesStream().count(); + } + + public long countAllVotes() { + return votes.size(); + } } From 57a26b22e56390f963514f870a6066f1a35f19a0 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 10:13:41 +0500 Subject: [PATCH 18/23] primitive-obsession: move votes without district counting to Votes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Продолжаю перемещать поведение при подсчете результата голосования в класс-коллекцию Votes. В этом коммите перенес подсчет голосов за официальных кандидатов без учета округов. Это позволило отказаться от хранения голосов без учета округов в простом массиве. --- .../main/java/org/elections/Elections.java | 25 ++++++++----------- java/src/main/java/org/elections/Vote.java | 12 +++++++++ java/src/main/java/org/elections/Votes.java | 10 ++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index e666061..8242d38 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -3,8 +3,6 @@ import java.util.*; public class Elections { - private static final String NO_DISTRICT = "No district"; - Map> votesWithoutDistrict; Map> votesWithDistricts; private final Electors electors; private final Candidates candidates; @@ -18,9 +16,6 @@ public Elections(Map> list, boolean withDistrict) { this.withDistrict = withDistrict; - votesWithoutDistrict = new HashMap<>(); - votesWithoutDistrict.put(NO_DISTRICT, new ArrayList<>()); - votesWithDistricts = new HashMap<>(); votesWithDistricts.put("District 1", new ArrayList<>()); votesWithDistricts.put("District 2", new ArrayList<>()); @@ -30,8 +25,6 @@ public Elections(Map> list, boolean withDistrict) { public void addOfficialCandidate(String candidate) { candidates.addOfficial(candidate); - votesWithoutDistrict.get(NO_DISTRICT).add(0); - votesWithDistricts.get("District 1").add(0); votesWithDistricts.get("District 2").add(0); votesWithDistricts.get("District 3").add(0); @@ -39,7 +32,10 @@ public void addOfficialCandidate(String candidate) { public void voteFor(String electorName, String candidateName, String electorDistrict) { if (!withDistrict) { - countVote(candidateName, votesWithoutDistrict, NO_DISTRICT); + if (candidates.isUnregistered(candidateName)) { + candidates.addUnofficial(candidateName); + } + //countVote(candidateName, votesWithoutDistrict, NO_DISTRICT); } else if (votesWithDistricts.containsKey(electorDistrict)) { countVote(candidateName, votesWithDistricts, electorDistrict); } @@ -70,17 +66,16 @@ public Map results() { long nbValidVotes = votes.countValid(); long nbVotes = votes.countAllVotes(); - Map officialCandidatesResult = new HashMap<>(); + Map officialCandidatesResult = new HashMap<>(); for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - officialCandidatesResult.put(candidates.get(i), 0); + officialCandidatesResult.put(candidates.get(i), 0L); } if (!withDistrict) { - for (Map.Entry> entry : votesWithoutDistrict.entrySet()) { - ArrayList districtResult = fillVoteBuckets(entry); + Map districtResultByCandidate = votes.resultForNoDistrict(); - for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - officialCandidatesResult.put(candidates.get(i), districtResult.get(i)); - } + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { + final String candidateName = candidates.get(i); + officialCandidatesResult.put(candidateName, districtResultByCandidate.getOrDefault(candidateName, 0L)); } for (int i = 0; i < officialCandidatesResult.size(); i++) { Fraction candidateResult = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), nbValidVotes); diff --git a/java/src/main/java/org/elections/Vote.java b/java/src/main/java/org/elections/Vote.java index f756a91..f71d9bc 100644 --- a/java/src/main/java/org/elections/Vote.java +++ b/java/src/main/java/org/elections/Vote.java @@ -22,4 +22,16 @@ public boolean hasCandidate() { public boolean isForUnofficialCandidate() { return !this.candidate.isOfficial(); } + + public boolean isForOfficialCandidate() { + return this.candidate.isOfficial(); + } + + public boolean isForDistrict(String district) { + return this.district.equals(district); + } + + public String candidateName(){ + return this.candidate.name(); + } } diff --git a/java/src/main/java/org/elections/Votes.java b/java/src/main/java/org/elections/Votes.java index d9863de..b534305 100644 --- a/java/src/main/java/org/elections/Votes.java +++ b/java/src/main/java/org/elections/Votes.java @@ -2,8 +2,12 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.stream.Stream; +import static java.util.stream.Collectors.counting; +import static java.util.stream.Collectors.groupingBy; + public class Votes { private final List votes; @@ -34,4 +38,10 @@ public long countValid() { public long countAllVotes() { return votes.size(); } + + public Map resultForNoDistrict() { + return votes.stream() + .filter(Vote::isForOfficialCandidate) + .collect(groupingBy(Vote::candidateName, counting())); + } } From 2d2fa34d4088905a82d7ee3ec1587dad868377fb Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 15:58:34 +0500 Subject: [PATCH 19/23] primitive-obsession: move votes with district counting to Votes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit При переносе процедуры определения результата голосования с учетом округов выяснилось, что собственно количество голосов не важно - важно имя кандидата-победителя в каждом округе. Это удалось отразить на уровне методов класса-коллекции Votes. --- .../main/java/org/elections/Elections.java | 27 +++++-------------- java/src/main/java/org/elections/Votes.java | 18 ++++++++++++- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 8242d38..7b1073e 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -1,6 +1,10 @@ package org.elections; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; public class Elections { Map> votesWithDistricts; @@ -83,14 +87,7 @@ public Map results() { } } else { for (Map.Entry> entry : votesWithDistricts.entrySet()) { - ArrayList districtResult = fillVoteBuckets(entry); - - int districtWinnerIndex = 0; - for (int i = 1; i < districtResult.size(); i++) { - if (districtResult.get(districtWinnerIndex) < districtResult.get(i)) - districtWinnerIndex = i; - } - final String districtWinner = candidates.get(districtWinnerIndex); + String districtWinner = votes.districtWinner(entry.getKey()); officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); } for (int i = 0; i < officialCandidatesResult.size(); i++) { @@ -111,16 +108,4 @@ public Map results() { return results; } - - private ArrayList fillVoteBuckets(Map.Entry> entry) { - ArrayList districtResult = new ArrayList<>(); - ArrayList districtVotes = entry.getValue(); - for (int i = 0; i < districtVotes.size(); i++) { - if (candidates.isOfficial(candidates.get(i))) { - districtResult.add(districtVotes.get(i)); - } - } - return districtResult; - } - } diff --git a/java/src/main/java/org/elections/Votes.java b/java/src/main/java/org/elections/Votes.java index b534305..441d772 100644 --- a/java/src/main/java/org/elections/Votes.java +++ b/java/src/main/java/org/elections/Votes.java @@ -40,8 +40,24 @@ public long countAllVotes() { } public Map resultForNoDistrict() { + return votesForOfficialCandidates().collect(groupingBy(Vote::candidateName, counting())); + } + + private Stream votesForOfficialCandidates() { return votes.stream() - .filter(Vote::isForOfficialCandidate) + .filter(Vote::isForOfficialCandidate); + } + + public String districtWinner(String districtName) { + return resultForDistrict(districtName).entrySet().stream() + .max(Map.Entry.comparingByValue()) + .map(Map.Entry::getKey) + .orElse(null); + } + + private Map resultForDistrict(String districtName) { + return votesForOfficialCandidates() + .filter(vote -> vote.isForDistrict(districtName)) .collect(groupingBy(Vote::candidateName, counting())); } } From 7a7f4797efcd65406a9a3cdb0644d3d40a8dd351 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Tue, 31 May 2022 16:11:04 +0500 Subject: [PATCH 20/23] primitive-obsession: remove the Map of votes by district MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Окончательный этап замены словарей и массивов для хранения результатов голосования на объект-коллекцию Votes. --- .../main/java/org/elections/Elections.java | 44 ++++--------------- java/src/main/java/org/elections/Votes.java | 3 +- 2 files changed, 10 insertions(+), 37 deletions(-) diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index 7b1073e..c865729 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -7,11 +7,11 @@ import java.util.Map; public class Elections { - Map> votesWithDistricts; private final Electors electors; private final Candidates candidates; private final Votes votes; private boolean withDistrict; + private final List districts = new ArrayList<>(); public Elections(Map> list, boolean withDistrict) { electors = Electors.fromMapByDistrict(list); @@ -20,51 +20,25 @@ public Elections(Map> list, boolean withDistrict) { this.withDistrict = withDistrict; - votesWithDistricts = new HashMap<>(); - votesWithDistricts.put("District 1", new ArrayList<>()); - votesWithDistricts.put("District 2", new ArrayList<>()); - votesWithDistricts.put("District 3", new ArrayList<>()); + districts.add("District 1"); + districts.add("District 2"); + districts.add("District 3"); } public void addOfficialCandidate(String candidate) { candidates.addOfficial(candidate); - - votesWithDistricts.get("District 1").add(0); - votesWithDistricts.get("District 2").add(0); - votesWithDistricts.get("District 3").add(0); } public void voteFor(String electorName, String candidateName, String electorDistrict) { - if (!withDistrict) { - if (candidates.isUnregistered(candidateName)) { - candidates.addUnofficial(candidateName); - } - //countVote(candidateName, votesWithoutDistrict, NO_DISTRICT); - } else if (votesWithDistricts.containsKey(electorDistrict)) { - countVote(candidateName, votesWithDistricts, electorDistrict); + if (candidates.isUnregistered(candidateName)) { + candidates.addUnofficial(candidateName); } + votes.registerVote(electorDistrict, electors.findByName(electorName), candidates.findByName(candidateName)); } - private void countVote(String candidate, Map> votesMap, String districtName) { - if (candidates.isUnregistered(candidate)) { - addUnofficialCandidate(candidate, votesMap); - } - incrementCandidateVotesCounter(candidate, candidates, votesMap.get(districtName)); - } - - private void addUnofficialCandidate(String candidate, Map> votesMap) { - candidates.addUnofficial(candidate); - votesMap.forEach((district, votes) -> votes.add(0)); - } - - private void incrementCandidateVotesCounter(String candidate, Candidates candidates, ArrayList votesByCandidate) { - int index = candidates.indexOf(candidate); - votesByCandidate.set(index, votesByCandidate.get(index) + 1); - } - public Map results() { Map results = new HashMap<>(); long nbValidVotes = votes.countValid(); @@ -86,8 +60,8 @@ public Map results() { results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); } } else { - for (Map.Entry> entry : votesWithDistricts.entrySet()) { - String districtWinner = votes.districtWinner(entry.getKey()); + for (String district : districts) { + String districtWinner = votes.districtWinner(district); officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); } for (int i = 0; i < officialCandidatesResult.size(); i++) { diff --git a/java/src/main/java/org/elections/Votes.java b/java/src/main/java/org/elections/Votes.java index 441d772..1ed3131 100644 --- a/java/src/main/java/org/elections/Votes.java +++ b/java/src/main/java/org/elections/Votes.java @@ -44,8 +44,7 @@ public Map resultForNoDistrict() { } private Stream votesForOfficialCandidates() { - return votes.stream() - .filter(Vote::isForOfficialCandidate); + return votes.stream().filter(Vote::isForOfficialCandidate); } public String districtWinner(String districtName) { From 3b6e4444daaec1f22bbc45d03901c3288cd50a85 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Wed, 1 Jun 2022 08:03:38 +0500 Subject: [PATCH 21/23] duplicated-code: use the dedicated object for storing the result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Инструкции по форматированию итогового значения по каждому кандидату и категориям ошибочных голосов повторялись несколько раз. Их можно инкапсулировать в специальном объекте, знающем, как нужно форматировать выходные значения. --- .../main/java/org/elections/Elections.java | 41 ++++++++----------- .../java/org/elections/FormattedResult.java | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 25 deletions(-) create mode 100644 java/src/main/java/org/elections/FormattedResult.java diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index c865729..c3365d9 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -40,46 +40,37 @@ public void voteFor(String electorName, String candidateName, String electorDist } public Map results() { - Map results = new HashMap<>(); + FormattedResult formattedResult = new FormattedResult(Locale.FRENCH, "%.2f%%"); long nbValidVotes = votes.countValid(); long nbVotes = votes.countAllVotes(); - Map officialCandidatesResult = new HashMap<>(); - for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - officialCandidatesResult.put(candidates.get(i), 0L); - } if (!withDistrict) { Map districtResultByCandidate = votes.resultForNoDistrict(); - for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - final String candidateName = candidates.get(i); - officialCandidatesResult.put(candidateName, districtResultByCandidate.getOrDefault(candidateName, 0L)); - } - for (int i = 0; i < officialCandidatesResult.size(); i++) { - Fraction candidateResult = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), nbValidVotes); - results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", candidateResult.asPercent())); + for (String candidate : candidates.officialCandidates()) { + Fraction candidateResult = Fraction.withNumeratorDenominator(districtResultByCandidate.getOrDefault(candidate,0L), nbValidVotes); + formattedResult.addCandidateResult(candidate, candidateResult); } + } else { + Map officialCandidatesResult = new HashMap<>(); + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { + officialCandidatesResult.put(candidates.get(i), 0L); + } for (String district : districts) { String districtWinner = votes.districtWinner(district); officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); } - for (int i = 0; i < officialCandidatesResult.size(); i++) { - Fraction ratio = Fraction.withNumeratorDenominator(officialCandidatesResult.get(candidates.get(i)), officialCandidatesResult.size()); - results.put(candidates.get(i), String.format(Locale.FRENCH, "%.2f%%", ratio.asPercent())); + for (String candidate : candidates.officialCandidates()) { + Fraction ratio = Fraction.withNumeratorDenominator(officialCandidatesResult.getOrDefault(candidate, 0L), districts.size()); + formattedResult.addCandidateResult(candidate, ratio); } } - Fraction blanks = Fraction.withNumeratorDenominator(votes.countBlanks(), nbVotes); - results.put("Blank", String.format(Locale.FRENCH, "%.2f%%", blanks.asPercent())); - - Fraction nulls = Fraction.withNumeratorDenominator(votes.countNulls(), nbVotes); - results.put("Null", String.format(Locale.FRENCH, "%.2f%%", nulls.asPercent())); - - int nbElectors = electors.size(); - Fraction abstentionResult = Fraction.withNumeratorDenominator(nbElectors - nbVotes, nbElectors); - results.put("Abstention", String.format(Locale.FRENCH, "%.2f%%", abstentionResult.asPercent())); + formattedResult.addBlankVotes(Fraction.withNumeratorDenominator(votes.countBlanks(), nbVotes)); + formattedResult.addNullVotes(Fraction.withNumeratorDenominator(votes.countNulls(), nbVotes)); + formattedResult.addAbstention(Fraction.withNumeratorDenominator(electors.size() - nbVotes, electors.size())); - return results; + return formattedResult.result(); } } diff --git a/java/src/main/java/org/elections/FormattedResult.java b/java/src/main/java/org/elections/FormattedResult.java new file mode 100644 index 0000000..10feace --- /dev/null +++ b/java/src/main/java/org/elections/FormattedResult.java @@ -0,0 +1,41 @@ +package org.elections; + +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +public class FormattedResult { + private final Locale locale; + private final String percentFormat; + private final Map result; + + public FormattedResult(Locale locale, String percentFormat) { + this.locale = locale; + this.percentFormat = percentFormat; + result = new HashMap<>(); + } + + public void addCandidateResult(String candidate, Fraction candidateResult) { + putFormattedValue(candidate, candidateResult); + } + + private void putFormattedValue(String key, Fraction value) { + result.put(key, String.format(locale, percentFormat, value.asPercent())); + } + + public void addBlankVotes(Fraction blanks) { + putFormattedValue("Blank", blanks); + } + + public void addNullVotes(Fraction nulls) { + putFormattedValue("Null", nulls); + } + + public void addAbstention(Fraction abstentionResult) { + putFormattedValue("Abstention", abstentionResult); + } + + public Map result() { + return new HashMap<>(this.result); + } +} From 20c290ad02d8659fef59fdf882f2fcbbde2469ca Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Wed, 1 Jun 2022 08:46:18 +0500 Subject: [PATCH 22/23] switch-statements: replace conditional result counting with strategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Это получился довольно большой рефакторинг, в ходе которого условная организация расчета итоговых результатов голосования была разделена на два класса-стратегии. Выбор конкретного класса осуществляется при создании класса Elections. --- .../main/java/org/elections/Elections.java | 42 ++++--------------- .../org/elections/NoDistrictVotingResult.java | 18 ++++++++ .../org/elections/VotingResultStrategy.java | 37 ++++++++++++++++ .../elections/WithDistrictVotingResult.java | 29 +++++++++++++ 4 files changed, 91 insertions(+), 35 deletions(-) create mode 100644 java/src/main/java/org/elections/NoDistrictVotingResult.java create mode 100644 java/src/main/java/org/elections/VotingResultStrategy.java create mode 100644 java/src/main/java/org/elections/WithDistrictVotingResult.java diff --git a/java/src/main/java/org/elections/Elections.java b/java/src/main/java/org/elections/Elections.java index c3365d9..e85f052 100644 --- a/java/src/main/java/org/elections/Elections.java +++ b/java/src/main/java/org/elections/Elections.java @@ -1,7 +1,6 @@ package org.elections; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -10,19 +9,21 @@ public class Elections { private final Electors electors; private final Candidates candidates; private final Votes votes; - private boolean withDistrict; - private final List districts = new ArrayList<>(); + private final VotingResultStrategy voting; public Elections(Map> list, boolean withDistrict) { electors = Electors.fromMapByDistrict(list); candidates = new Candidates(); votes = new Votes(); - this.withDistrict = withDistrict; - + List districts = new ArrayList<>(); districts.add("District 1"); districts.add("District 2"); districts.add("District 3"); + + voting = withDistrict + ? new WithDistrictVotingResult(candidates, electors, districts) + :new NoDistrictVotingResult(candidates, electors); } public void addOfficialCandidate(String candidate) { @@ -41,36 +42,7 @@ public void voteFor(String electorName, String candidateName, String electorDist public Map results() { FormattedResult formattedResult = new FormattedResult(Locale.FRENCH, "%.2f%%"); - long nbValidVotes = votes.countValid(); - long nbVotes = votes.countAllVotes(); - - if (!withDistrict) { - Map districtResultByCandidate = votes.resultForNoDistrict(); - - for (String candidate : candidates.officialCandidates()) { - Fraction candidateResult = Fraction.withNumeratorDenominator(districtResultByCandidate.getOrDefault(candidate,0L), nbValidVotes); - formattedResult.addCandidateResult(candidate, candidateResult); - } - - } else { - Map officialCandidatesResult = new HashMap<>(); - for (int i = 0; i < candidates.officialCandidatesCount(); i++) { - officialCandidatesResult.put(candidates.get(i), 0L); - } - for (String district : districts) { - String districtWinner = votes.districtWinner(district); - officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); - } - for (String candidate : candidates.officialCandidates()) { - Fraction ratio = Fraction.withNumeratorDenominator(officialCandidatesResult.getOrDefault(candidate, 0L), districts.size()); - formattedResult.addCandidateResult(candidate, ratio); - } - } - - formattedResult.addBlankVotes(Fraction.withNumeratorDenominator(votes.countBlanks(), nbVotes)); - formattedResult.addNullVotes(Fraction.withNumeratorDenominator(votes.countNulls(), nbVotes)); - formattedResult.addAbstention(Fraction.withNumeratorDenominator(electors.size() - nbVotes, electors.size())); - + voting.count(votes).printOn(formattedResult); return formattedResult.result(); } } diff --git a/java/src/main/java/org/elections/NoDistrictVotingResult.java b/java/src/main/java/org/elections/NoDistrictVotingResult.java new file mode 100644 index 0000000..430a111 --- /dev/null +++ b/java/src/main/java/org/elections/NoDistrictVotingResult.java @@ -0,0 +1,18 @@ +package org.elections; + +import java.util.Map; +import java.util.stream.Collectors; + +public class NoDistrictVotingResult extends VotingResultStrategy { + public NoDistrictVotingResult(Candidates candidates, Electors electors) { + super(candidates, electors); + } + + @Override + protected Map calculateResult() { + long validVotes = votes.countValid(); + return votes.resultForNoDistrict().entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> Fraction.withNumeratorDenominator(e.getValue(), validVotes))); + } + +} diff --git a/java/src/main/java/org/elections/VotingResultStrategy.java b/java/src/main/java/org/elections/VotingResultStrategy.java new file mode 100644 index 0000000..d8df43a --- /dev/null +++ b/java/src/main/java/org/elections/VotingResultStrategy.java @@ -0,0 +1,37 @@ +package org.elections; + +import java.util.Map; + +public abstract class VotingResultStrategy { + protected final Candidates candidates; + protected final Electors electors; + protected Map result; + protected Votes votes; + + public VotingResultStrategy(Candidates candidates, Electors electors) { + this.candidates = candidates; + this.electors = electors; + } + + public void printOn(FormattedResult formattedResult) { + Fraction zero = Fraction.withNumeratorDenominator(0L,1L); + long nbValidVotes = votes.countValid(); + long nbVotes = votes.countAllVotes(); + + for (String candidate : candidates.officialCandidates()) { + formattedResult.addCandidateResult(candidate, result.getOrDefault(candidate, zero)); + } + + formattedResult.addBlankVotes(Fraction.withNumeratorDenominator(votes.countBlanks(), nbVotes)); + formattedResult.addNullVotes(Fraction.withNumeratorDenominator(votes.countNulls(), nbVotes)); + formattedResult.addAbstention(Fraction.withNumeratorDenominator(electors.size() - nbVotes, electors.size())); + } + + public VotingResultStrategy count(Votes votes) { + this.votes = votes; + this.result = calculateResult(); + return this; + } + + protected abstract Map calculateResult(); +} diff --git a/java/src/main/java/org/elections/WithDistrictVotingResult.java b/java/src/main/java/org/elections/WithDistrictVotingResult.java new file mode 100644 index 0000000..42b9973 --- /dev/null +++ b/java/src/main/java/org/elections/WithDistrictVotingResult.java @@ -0,0 +1,29 @@ +package org.elections; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public class WithDistrictVotingResult extends VotingResultStrategy { + private final List districts; + + public WithDistrictVotingResult(Candidates candidates, Electors electors, List districts) { + super(candidates, electors); + this.districts = districts; + } + + @Override + protected Map calculateResult() { + Map officialCandidatesResult = new HashMap<>(); + for (int i = 0; i < candidates.officialCandidatesCount(); i++) { + officialCandidatesResult.put(candidates.get(i), 0L); + } + for (String district : districts) { + String districtWinner = votes.districtWinner(district); + officialCandidatesResult.put(districtWinner, officialCandidatesResult.get(districtWinner) + 1); + } + return officialCandidatesResult.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> Fraction.withNumeratorDenominator(e.getValue(), districts.size()))); + } +} From 9de9d109b7dbdec914e30aaf962c1b8565f12304 Mon Sep 17 00:00:00 2001 From: nergal-perm Date: Wed, 1 Jun 2022 08:53:05 +0500 Subject: [PATCH 23/23] no-smell: cleaning unused methods and variables, formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit В общем, основной объем рефакторинга можно считать завершенным. Код явно стал менее связанным, как минимум структуры непосредственного хранения данных теперь инкапсулированы в объектах и спрятаны за интерфейсами этих объектов, поэтому их можно заменить на любые другие. Бизнес-логика теперь тоже видна гораздо лучше, алгоритмы подсчета голосов инкапсулированы в стратегиях подсчета. В ходе рефакторинга, кстати, был обнаружен и исправлен баг подсчета голосов, который было трудно обнаружить просто глядя на код. --- java/src/main/java/org/elections/Candidates.java | 16 +--------------- java/src/main/java/org/elections/Electors.java | 3 +-- java/src/main/java/org/elections/Fraction.java | 4 ++-- java/src/main/java/org/elections/Vote.java | 2 +- .../java/org/elections/VotingResultStrategy.java | 3 +-- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/java/src/main/java/org/elections/Candidates.java b/java/src/main/java/org/elections/Candidates.java index 561737a..9100604 100644 --- a/java/src/main/java/org/elections/Candidates.java +++ b/java/src/main/java/org/elections/Candidates.java @@ -16,15 +16,6 @@ public void addUnofficial(String name) { candidates.add(Candidate.unofficial(name)); } - public int indexOf(String candidate) { - for (int i = 0; i < candidates.size(); i++) { - if (candidate.equals(candidates.get(i).name())) { - return i; - } - } - return -1; - } - public List officialCandidates() { return candidates.stream() .filter(Candidate::isOfficial) @@ -36,10 +27,6 @@ public long officialCandidatesCount() { return candidates.stream().filter(Candidate::isOfficial).count(); } - public boolean isOfficial(String candidate) { - return candidates.stream().filter(sameNameAs(candidate)).anyMatch(Candidate::isOfficial); - } - public boolean isUnregistered(String candidate) { return candidates.stream().noneMatch(sameNameAs(candidate)); } @@ -55,7 +42,6 @@ public String get(int index) { public Candidate findByName(String candidateName) { return candidates.stream() .filter(candidate -> candidate.hasName(candidateName)) - .findFirst() - .orElse(null); + .findFirst().orElse(null); } } diff --git a/java/src/main/java/org/elections/Electors.java b/java/src/main/java/org/elections/Electors.java index 402f70f..d283ee8 100644 --- a/java/src/main/java/org/elections/Electors.java +++ b/java/src/main/java/org/elections/Electors.java @@ -3,7 +3,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.Predicate; import java.util.stream.Collectors; public class Electors { @@ -15,7 +14,7 @@ private Electors() { public static Electors fromMapByDistrict(Map> electorsByDistrict) { Electors instance = new Electors(); - for(Map.Entry> district : electorsByDistrict.entrySet()) { + for (Map.Entry> district : electorsByDistrict.entrySet()) { instance.addAll(district.getValue().stream() .map(el -> Elector.withDistrict_Name_(district.getKey(), el)) .collect(Collectors.toList())); diff --git a/java/src/main/java/org/elections/Fraction.java b/java/src/main/java/org/elections/Fraction.java index 1fb9710..31dee8e 100644 --- a/java/src/main/java/org/elections/Fraction.java +++ b/java/src/main/java/org/elections/Fraction.java @@ -14,9 +14,9 @@ private Fraction(long numerator, long denominator) { } public float asPercent() { - if (this.denominator == 0) { + if (this.denominator==0) { return 0; } - return (float)this.numerator * 100 / this.denominator; + return (float) this.numerator * 100 / this.denominator; } } diff --git a/java/src/main/java/org/elections/Vote.java b/java/src/main/java/org/elections/Vote.java index f71d9bc..fc83e6d 100644 --- a/java/src/main/java/org/elections/Vote.java +++ b/java/src/main/java/org/elections/Vote.java @@ -31,7 +31,7 @@ public boolean isForDistrict(String district) { return this.district.equals(district); } - public String candidateName(){ + public String candidateName() { return this.candidate.name(); } } diff --git a/java/src/main/java/org/elections/VotingResultStrategy.java b/java/src/main/java/org/elections/VotingResultStrategy.java index d8df43a..353d6db 100644 --- a/java/src/main/java/org/elections/VotingResultStrategy.java +++ b/java/src/main/java/org/elections/VotingResultStrategy.java @@ -14,8 +14,7 @@ public VotingResultStrategy(Candidates candidates, Electors electors) { } public void printOn(FormattedResult formattedResult) { - Fraction zero = Fraction.withNumeratorDenominator(0L,1L); - long nbValidVotes = votes.countValid(); + Fraction zero = Fraction.withNumeratorDenominator(0L, 1L); long nbVotes = votes.countAllVotes(); for (String candidate : candidates.officialCandidates()) {