По какому пути вы бы пошли при рефакторинге?

Привет пытаюсь разобраться с SOLID и конкретно с принципом открытости закрытости, который рекомендует расширять классы для добавления новой функциональности.

Задача: есть класс Balance у него есть метод getBalance который возвращает баланс пользователя без округления те с 10 знаков после запятой.

Менеджер проекта решил возвращать с округлением.

Как бы вы сделали и почему ?
1) Добавить округление в с существующий метод getBalance, запустить тесты на getBalance и через 5 мин пойти пить кофе.

2) Добавить в Класс Balance новый метод getAdvancedBalance c округлением а текущий метод не трогать тк у нас банк
и если что то сломается будет не приятно

3) Сделать новый класс AdvancedBalance унаследовать от Balance и переопределить метод getBalance,
отрефакторить весь проект на использование AdvancedBalance:getBalance вместо Balance:getBalance и молится что ничего не сломалось
хоть тесты и есть, но банк и если у когото олигарха что то не то отобразится вам мало не покажется

4) Сделать новый класс AdvancedBalance унаследовать от Balance и создать новый метод getAdvancedBalance, отрефакторить
и снова молится.
  • Вопрос задан
  • 633 просмотра
Пригласить эксперта
Ответы на вопрос 7
xmoonlight
@xmoonlight
https://sitecoder.blogspot.com
Ни один из перечисленных!
Добавить необязательный параметр для задания формата возвращаемого значения в метод getBalance и всё.
Ответ написан
@EvgeniiR
https://github.com/EvgeniiR
3) Сделать новый класс AdvancedBalance унаследовать от Balance и переопределить метод getBalance

4) Сделать новый класс AdvancedBalance унаследовать от Balance и создать новый метод getAdvancedBalance,

Ох уж эти ООП-девелоперы. Ради чего вы там наследовать собрались?
Добавьте интерфейс NumberFormatter или BalanceFormatter, прогоняйте числа через них в нужных местах.

молится что ничего не сломалось

хоть тесты и есть

Они не просто "есть", их ещё и поддерживать нужно. Пойдите по пути написания тестов на места которые могут поломаться.

с принципом открытости закрытости, который рекомендует расширять классы для добавления новой функциональности.

Расширять, а не наследовать, принцип о том чтобы менять как можно меньше кода, в идеале вообще не трогать, а вы, якобы следуя ему, предлагаете пол проекта перелопатить для использования другого интерфейса

p.s. Гляньте, например, https://www.youtube.com/watch?v=pu0EXQvoaCc
Ответ написан
@vanyamba-electronics
Метод getBalance возвращает одно число, метод getBalanceOkrugl будет возвращать другое число.
Это подобно тому, как если бы getBalance возвращал простые числа, а менеджер сказал, что теперь нужно работать с числами Фибоначчи. Это два разных множества.
Ответ написан
h0w4rd
@h0w4rd
Junior Node.js dev.
2) Добавить в Класс Balance новый метод getAdvancedBalance c округлением а текущий метод не трогать тк у нас банк и если что то сломается будет не приятно

Ведь в некоторых местах может понадобится получить баланс БЕЗ округления.
Ответ написан
@vism
Идеально решение как написал Евгений Ромашкан
Делаете BalanceFormatter и используете как желаете его.
главное не забудте сразу семантику заложит правильную, а то любят люди такое пихать в папку Хелперс и потом сиди копайся в помойке "хелперов"
Ответ написан
vfreelancer
@vfreelancer
php
почему добавлять параметр со значению по умолчанию плохо, объяснено у Мартина в Чистом коде. это действительно ерундовое решение
Ответ написан
@ddd329
Менеджер проекта решил возвращать с округлением.

Обычно округление и форматирование значений с плавающей запятой требуется для отображений в графическом интерфейсе пользователя.
Если вашему менеджеру необходимо именно это, то никакие изменения в класс Balanceвносить не нужно, т.к. этот класс является бизнес-сущностью и соответственно реализован в бизнес-слое, а логику отображения необходимо править в слое отображения.
Например, вы используете паттерн Model-ViewModel-View, то эту задачу должен решать класс ViewModel. Если используете паттерн Model-View-Presenter или Model-View-Controller, то соответственно задачу решает Presenter и Controller.
Если речь об этом, то в принципе разговор здесь можно закончить, но если это необходимо для решения бизнес-задач, то давайте разберемся по порядку.

1) Добавить округление в с существующий метод getBalance, запустить тесты на getBalance и через 5 мин пойти пить кофе.

На вашем месте я бы сделал именно это, если речь идет не об отображении данных.

2) Добавить в Класс Balance новый метод getAdvancedBalance c округлением а текущий метод не трогать тк у нас банки если что то сломается будет не приятно

Вот здесь очень большую неразбериху в код внесете, у клиента получается два вида баланса что ли? Так какой из них вызывать чтобы узнать баланс? Если вы хотите первый метод оставить, значит он вам нужен... Вообщем нет..

3) Сделать новый класс AdvancedBalance унаследовать от Balance и переопределить метод getBalance, отрефакторить весь проект на использование AdvancedBalance:getBalance вместо Balance:getBalance и молится что ничего не сломалось хоть тесты и есть, но банк и если у когото олигарха что то не то отобразится вам мало не покажется

Можно так делать, но не нужно в вашем случае. Особенно не очень хорошо, как вы сказали чтобы отрефакторить весь проект и произвести замену класса Balance на AdvancedBalance. Вам необходим выполнить маленькую простую задачу, а изменений в этом случае придется сделать очень много, а это большой риск для внесения ошибок.
Вообще для таких решений код проектируют таким образом: класс Balance объявляют абстрактным и у него определяют статический фабричный метод var balance = Balance.Create(/* агрументы */). Ну и соответственно в зависимости от значений входящих аргументов, вам вернется правильный наследник. Если вы захотите добавить нового наследника, например AdvancedBalance, то внесете изменения только в метод Create. Вот здесь наверное будет соблюден принцип открытости/закрытости.

4) Сделать новый класс AdvancedBalance унаследовать от Balance и создать новый метод getAdvancedBalance, отрефакторить
и снова молится.

Если вы так сделаете, то нарушите принцип Барбары Лисков.
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы