vincent15000's avatar

Refactoring problem around bills

Hello,

I have to refactor a code that generates bills, each bill is composed of several bill types.

  • all bill types are calculated with the same logic, but with some little differences

$bill_type_1, $bill_type_2, $bill_type_3, $bill_type_4

same logic => retrieve the dates, calculate the duration, retrieve the price list, calculate the amount

little differences => retrive the dates, calculate the duration, round the duration (not for all bill types), retrieve the price list, calculate the amount, round the amount (not for all bill types)

  • each bill type calculates an amount

$bill_type_1_amount = $bill_type_1->calculate();

  • some bills can be calculated isolated from the others, but some other bills need the amount from previous bills to calculate the amount

$bill_type_3_amount = $bill_type_3->calculate($bill_type_1_amount + $bill_type_2_amount);

What idea I have to refactor the code, given that there are some little differences between each bill, is to create a factory with an abstract class or an interface and implement each difference in each bill type's class. But I don't know for the moment where to put the logic code (that is the same for each bill type).

Do you have any better idea ?

And to share the datas that have to be used by the calculate() methods, what would be the better idea ? session ? database ? global variable in a parent class ? something else ?

If you can help me to find the better design for this problem, it would be great ;).

Thanks a lot.

Vincent

0 likes
8 replies
aletopo's avatar

Hi @vincent15000

There are two options in my opinion:

  • Interface + Abstract Class + Class
  • Interface + Trait + Class

Talking about "types" of bills is a kind of inheritance. But if you want to share (not just inherit) some behavior between classes, PHP Traits could be an option to achieve that, in order to avoid too much inheritance levels.

You say given that there are some little differences between each bill - That is a typical case to apply an abstract class and override in concrete classes.

create a factory - Factory purpose is hide the way to create objects. I am not sure that is the problem that you describe.

with an abstract class or an interface - Design patterns like Factory are useful with interfaces, more than abstract classes.

But I don't know for the moment where to put the logic code (that is the same for each bill type) - Maybe a Trait ?

And to share the datas that have to be used by the calculate() methods - Use object values to share data and inject that or just one method calling to other to get the value.

  • Session - Not recommended. Session is for sharing data between requests, not between objects.
  • Database - May be, but an application should be capable of work even without data storage implementation.
  • Global variable in a parent class - Are you going to inherit objects, just to share values ? Mmm...
1 like
vincent15000's avatar

@aletopo Thank you for your answer.

Not sure to understand all what you said to me about the factories.

Interface + abstract class + class OR interface + trait + class => isn't this architecture typical of factories ?

And about sharing datas, what could be the best way if no one of my ideas is relevant ?

Perhaps I have to specify that the initial datas necessary to calculate the amounts come from the database. So the logic is to retrieve the initial datas like : dates and some specific informations from the customer to check parameters which will be used to calculate all bill types. That means => I'd like to avoid to pass these initial data into each calculate() function and share them directly with all bill types classes. That's why I thought about a parent class only responsible for sharing these initial datas. And all along the calculations for each bill type, some properties of this parent class could be updated with new values, and finally used by any bill types classes when needed. But perhaps it's not a good idea ?

And there is perhaps a problem because some bill types classes are dependant with other bill types classes, I mean for example that bill type 4 needs the result of bill type 1 so that it can be calculated.

Well ... that's not simple ... but I really need to find a way to share these datas.

jlrdw's avatar

@vincent15000 I don't go for all those patterns.

I would simply make a class that has the correct if statements needed and return what's needed from the call.

A helper or service class, really doesn't matter which.

1 like
vincent15000's avatar

@jlrdw That's how the code is for the moment. But it's maintanable with difficulty and very difficult to read. That's why I'd like to find another way to organize the code. Perhaps here the best is the enemy of good.

1 like
jlrdw's avatar
jlrdw
2 years ago
Best Answer
Level 75

@vincent15000 Have you looked into some classes in the framework and symfony as well, of course a class can be hard to read.

See https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/HttpFoundation/Request.php

That's why we are developers, we make complex classes.

By the way, laravel uses that class.

In your case, I'd make static methods, easier to call and deal with.

Edit:

Or instance and use __callstatic. Which is the basics of a facade.

1 like
vincent15000's avatar

@jlrdw I didn't specify, but it's a pure PHP code, not a Laravel one. But it doesn't change the problem.

Sure classes can be complex.

My code works, but I'm not satisfied of it. Perhaps it's just normal that it's complex and I should accept it as it is.

1 like
jlrdw's avatar

@vincent15000 to me that's what classes are for, that way you keep your controllers clean.

In one application I have some very complex search conditions, rather than doing all of the request in the controller I wrote a special little class to handle this and return the final search conditions.

Edit:

But these are just my thoughts on the topic.

1 like

Please or to participate in this conversation.