V
V
vrazbros2019-09-12 11:30:51
Software design
vrazbros, 2019-09-12 11:30:51

Which way would you go when refactoring?

Hello, I'm trying to figure out SOLID and specifically the open-closed principle, which recommends extending classes to add new functionality.
Task: there is a Balance class , it has a getBalance method that returns the user's balance without rounding those with 10 decimal places.
The project manager decided to return with rounding.
How would you do it and why?
1) Add rounding to the existing getBalance method, run tests on getBalance and after 5 minutes go drink coffee.
2) Add a new getAdvancedBalance method with rounding to the Balance class, and do not touch the current method because we have a bank
and if something breaks it will not be pleasant
3) Make a new class AdvancedBalance inherit from Balance and override the getBalance method,
refactor the entire project to use AdvancedBalance:getBalance instead of Balance:getBalance and pray that nothing is broken
, even though there are tests, but the bank and if someone oligarch has something wrong will be displayed to you 4) Make a new AdvancedBalance class inherit from Balance and create a new getAdvancedBalance
method, refactor
and pray again.

Answer the question

In order to leave comments, you need to log in

7 answer(s)
X
xmoonlight, 2019-09-12
@xmoonlight

None of the above!
Add an optional parameter to set the return value format to the getBalance method and that's it.

E
Evgeny Romashkan, 2019-09-12
@EvgeniiR

3) Make a new class AdvancedBalance inherit from Balance and override the getBalance method

Oh, those OOP developers. Why are you going to inherit there?
Add the NumberFormatter or BalanceFormatter interface, run numbers through them in the right places.
They are not just "existing", they also need to be supported. Take the path of writing tests for places that can break.
Extend , not inherit, the principle of changing as little code as possible, ideally not touching it at all, and you, supposedly following it, suggest shoveling half of the project to use a different interface
ps Look, for example, https://www.youtube. com/watch?v=pu0EXQvoaCc

V
vanyamba-electronics, 2019-09-13
@vanyamba-electronics

The getBalance method returns one number, the getBalanceOkrugl method will return a different number.
It's like getBalance returning prime numbers, and the manager said that now we need to work with Fibonacci numbers. These are two different sets.

H
h0w4rd, 2019-09-12
@h0w4rd

2) Add a new getAdvancedBalance method with rounding to the Balance class, and do not touch the current method because we have a bank and if something breaks it will not be pleasant

Indeed, in some places it may be necessary to obtain a balance WITHOUT rounding.

V
vism, 2019-09-13
@vism

An ideal solution as Evgeny Romashkan
wrote . Make a BalanceFormatter and use it as you wish.
the main thing is not to forget immediately the semantics will be laid down correctly, otherwise people like to shove such things into the Helpers folder and then sit and dig in the trash of "helpers"

V
vfreelance, 2019-09-13
@vfreelancer

why adding a parameter with a default value is bad, explained by Martin in Clean Code. this is a really stupid decision.

D
ddd329, 2019-09-20
@ddd329

The project manager decided to return with rounding.

Generally, rounding and formatting of floating point values ​​is required for GUI displays.
If this is exactly what your manager needs, then no changes need to be made to the Balance class , because this class is a business entity and is accordingly implemented in the business layer, and the display logic must be edited in the display layer. For example, if you are using the Model-ViewModel-View
pattern , then the ViewModel class should handle this task . If you use the Model-View-Presenter or Model-View-Controller pattern, then the Presenter and Controller solve the problem, respectively .
If we are talking about this, then in principle the conversation can be ended here, but if it is necessary to solve business problems, then let's take a look at it in order.
If I were you, I would do exactly that, if it's not about displaying data.
Here you will introduce a very big mess into the code, the client gets two types of balance or what? So which one to call to find out the balance? If you want to leave the first method, then you need it ... Actually, no ..
You can do this, but it's not necessary in your case. Especially not very good, as you said, to refactor the entire project and replace the Balance class with AdvancedBalance. You need to complete a small simple task, and in this case, there will be a lot of changes, and this is a big risk for introducing errors.
In general, for such solutions, the code is designed in this way: the Balance class is declared abstract and it has a static factory method defined var balance = Balance.Create(/* агрументы */). Well, and accordingly, depending on the values ​​of the incoming arguments, the correct successor will be returned to you. If you want to add a new child, such as AdvancedBalance, you will only change the Create method. This is where the principle of openness/closeness will probably be observed.
4) Make a new AdvancedBalance class inherit from Balance and create a new getAdvancedBalance method, refactor
and pray again.

If you do so, you are violating the Barbara Liskov principle.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question