Open
Conversation
Две строки кода, добавляющие голос за конкретного кандидата путем увеличения счетчика в соответствующем элементе массива голосов (по индексу кандидата). В целом это решение уже выглядит совсем не ООП, потому что использует примитивы для подсчета голосов и простые массивы для хранения счетчиков. Вместо этого можно использовать объекты Vote и специализированные объекты-коллекции для хранения счетчиков. Мне совершенно непонятно, каким образом можно перейти к ООП-решению из этого конкретного примера. Поэтому я просто выделю метод, содержащий эти две строки кода. Сначала выделю временные переменные - они станут аргументами нового метода, затем применю стандартный рефакторинг Extract Method. IDE сама предложит мне заменить второй случай дублирования кода.
Код в методе voteFor в ветках, обрабатывающих случай отсутствия кандидата в списках, содержит Magic Literals и значительно отличается в части добавления информации о новом голосе. Однако по сути это одна и та же операция, состоящая из двух частей: добавлении нового счетчика для голосов кандидата и увеличение этого счетчика на единицу. Намек на это дает разный способ добавления нового элемента в массив счетчиков голосов для случаев с учетом и без учета округов. Без учета округов (т.е. при наличии только одного массива счетчиков) новый счетчик добавляется сразу с единицей в качестве начального значения. С учетом округов сначала в массивы счетчиков для каждого округа добавляются новые счетчики с начальным значением 0, а затем нужный (соответствующий текущему округу) счетчик увеличивается на единицу. Чтобы сделать две ветки кода максимально похожими, нужно разделить операцию добавления счетчика в ветке без учета округов на две: добавление с нулем в качестве начального значения и увеличение счетчика (отдельный метод, выделенный в предыдущем рефакторинге).
В методе voteFor ветки кода, обрабатывающие голос за ранее добавленного кандидата, содержат только одну инструкцию - увеличение счетчика. Эта же инструкция содержится и в ветках, обрабатывающих добавление нового кандидата. Инструкцию можно вынести за рамки условного оператора и удалить пустую ветку.
Операция добавления счетчика для нового кандидата для случаев с учетом и без учета округов выглядит практически одинаково, разница лишь в том, в какой тип коллекции нужно внести изменения. Можно обобщить случай без учета округов и представить массив счетчиков в виде словаря с единственной записью. Ключ этой записи - фиктивное название округа, а значение - массив счетчиков. Точно такая же коллекция используется для хранения счетчиков с учетом округов. Для этого я сначала добавил новое поле для хранения словаря счетчиков без учета округов. Затем продублировал все операции увеличения счетчиков, добавив к ним инструкцию добавления в новый словарь. В методе определения результата голосования ввел временную переменную для хранения массива счетчиков без учета округов, чтобы остальной код метода продолжал работать с массивом. В конце заменил значение этой переменной на значение из единственной записи нового словаря, а старое поле класса с массивом счетчиков без учета округов удалил.
Содержимое веток, обрабатывающих добавление нового кандидата при регистрации голосов, практически одинаково для случаев с учетом и без учета округов, поэтому можно выделить его в небольшой метод. Попутно выяснилось, что с точки зрения бизнес-логики такой кандидат (добавленный в ходе регистрации голосов) не является официально зарегистрированным и не входит в массив официальных кандидатов. Поэтому уже существующий метод создания кандидата до регистрации голосов был переименован в `addOfficialCandidate`.
Ветки метода `voteFor` теперь выглядят идентично, разница только в принимаемых аргументах. Можно легко применить "Выделение метода". В целом, все рефакторинги, начиная с коммита d3a78ef, оказались частью более крупного рефакторинга Decompose Conditional. Код по-прежнему очень процедурный, в ходе рефакторингов не появились новые объекты. С точки зрения разделения кода на Данные, Вычисления и Действия тоже ничего не поменялось - код все так же использует поля класса, которые в данном случае практически являются глобальными переменными.
Тип коллекции, используемой в подсчете общего количества голосов в случае без учета округов, можно обобщить до словаря (как в одном из предыдущих рефакторингов). После этого функция подсчета будет идентичной в обоих случаях, так что ее можно вынести в отдельный метод. Кроме того, собственно расчет общего количества голосов можно перенести ближе к месту его непосредственного использования при расчете итоговых статистик и форматировании результатов.
Тип коллекции, используемой в подсчете количества голосов за официальных кандидатов в случае без учета округов, можно обобщить до словаря (как в одном из предыдущих рефакторингов). После этого функция подсчета будет идентичной в обоих случаях, так что ее можно вынести в отдельный метод.
Это подготовительный рефакторинг, мотивированный тем, что большая часть процедуры расчета результатов - это классификация голосов на голоса за кандидата (с учетом округа или без), пустые бланки (empty) и ошибочные бланки (null). Однако выделить классификацию в отдельный метод с наскоку не получилось из-за того, что число пустых и ошибочных бланков хранится в примитиве Integer, а значит, должно быть возвращаемым из метода значением. Поскольку в методе будут рассчитываться три категории голосов, то и возвращаемых значений должно быть три. Поскольку в Java метод может возвращать только одно значение, нужно остальные два аккумулировать в каком-то объекте, передаваемом в метод по ссылке. Таким объектом вполне может стать объект Fraction, в чьи обязанности будет входить расчет процентного значения для каждой из категорий голосов.
В итоге после использования аккумулирующих объектов Fraction для подсчета пустых и ошибочных бланков функция категоризации бланков стала практически идентичной для случаев с учетом и без учета округов. Поэтому можно выделить ее в новый метод fillVoteBuckets.
Я пока не знаю, куда двинуться дальше с рефакторингом метода подсчета результатов голосования. Код явно очень сильно зависит от структур данных, это проявление запаха "Primitive Obsession". Зависимостей от конкретной структуры данных очень много, проще всего было инкапсулировать информацию о списке избирателей в специальном классе-коллекции Electors. В дальнейшем это поможет реализовать требование о пометке голоса, в котором округ не соответствует округу избирателя, как ошибочного.
Следующий шаг - инкапсуляция списка кандидатов в выделенном классе. Полностью перейти на использование нового объекта не получилось, в этом коммите реализован отказ от использования отдельного списка официально зарегистрированных кандидатов. Вместо него все обращения идут к новому классу-коллекции Candidates.
Второй этап замены простого списка кандидатов на специализированный объект-коллекцию Candidates.
Не знаю, к какой категории запахов и рефакторингов отнести эти изменения. Суть их заключается в том, что созданный класс Vote обладает всей информацией для того, чтобы определить свой тип (пустой или ошибочный в рамках данного коммита). Соответственно, подсчет количества таких голосов можно смело переместить из цикла, обрабатывающего голоса, в коллекцию Votes. В результате стало возможным отказаться от использования объектов Fraction в качестве "накопителей" при подсчете и сделать их по-настоящему неизменяемыми (с final полями).
Все подсчеты количества голосов разных типов стало возможно перенести в класс-коллекцию Votes. В результате оказались не нужны многословные методы, подсчитывавшие голоса в циклах и с помощью полей класса (по сути, глобальных переменных)
Продолжаю перемещать поведение при подсчете результата голосования в класс-коллекцию Votes. В этом коммите перенес подсчет голосов за официальных кандидатов без учета округов. Это позволило отказаться от хранения голосов без учета округов в простом массиве.
При переносе процедуры определения результата голосования с учетом округов выяснилось, что собственно количество голосов не важно - важно имя кандидата-победителя в каждом округе. Это удалось отразить на уровне методов класса-коллекции Votes.
Окончательный этап замены словарей и массивов для хранения результатов голосования на объект-коллекцию Votes.
Инструкции по форматированию итогового значения по каждому кандидату и категориям ошибочных голосов повторялись несколько раз. Их можно инкапсулировать в специальном объекте, знающем, как нужно форматировать выходные значения.
Это получился довольно большой рефакторинг, в ходе которого условная организация расчета итоговых результатов голосования была разделена на два класса-стратегии. Выбор конкретного класса осуществляется при создании класса Elections.
В общем, основной объем рефакторинга можно считать завершенным. Код явно стал менее связанным, как минимум структуры непосредственного хранения данных теперь инкапсулированы в объектах и спрятаны за интерфейсами этих объектов, поэтому их можно заменить на любые другие. Бизнес-логика теперь тоже видна гораздо лучше, алгоритмы подсчета голосов инкапсулированы в стратегиях подсчета. В ходе рефакторинга, кстати, был обнаружен и исправлен баг подсчета голосов, который было трудно обнаружить просто глядя на код.
nergal-perm
force-pushed
the
logging-my-refactoring
branch
from
1dde11f to
9de9d10
Compare
nergal-perm
force-pushed
the
inquiry-tests
branch
from
0d76e4e to
ebc4f08
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.