Loose connection against encapsulation. The best approach to balanced design

Question

In accordance with the following examples:

class InvoiceGenerator { function create(Invoice $invoice) { $invoice->create(); } } class InvoiceGenerator { function create($invoiceData) { $invoice = new Invoice(); $invoice->create($invoiceData); } } 

The first example has less connection between the InvoiceGenerator and Invoice classes, because InvoiceGenerator does not require the Invoice class. In addition, it can handle not only a class, but also an entire interface with very little modification. I have read this principle many times, which reads: code for an interface, not an implementation. The disadvantage of this case is that I am forced to create an instance of the Invoice class in client code.

The second has more encapsulation . The entire process of creating an instance and creating an invoice is delegated to the InvoiceGenerator class. Despite the fact that both classes are connected, it makes sense because the “invoice generator” will not do anything without invoices.

What do you think is most suitable? Or what are the key points for a balanced design between them?

Answer 1

First of all, I think you are mixing some things. It looks like your class is an InvoiceGenerator factory class . His responsibility is to create and return an invoice object. An Invoice object is not an InvoiceGenerator dependency (in this case, it would be better to pass an Invoice object as an argument, known as Injection Dependency ).

However, as I said, the InvoiceGenerator class is represented by the factory class, and the whole idea of ​​the factory class is that the logic for creating an object instance is encapsulated inside this class (as in your second example). If you pass the Invoice object as an argument (as in the first example), you will need to create it somewhere else, and if there are several places in your code where this will happen, you will end up duplicating the logic of creating the instance. The whole idea of ​​the factory pattern is to prevent this.

Using the factory class, you can encapsulate the object creation logic and avoid duplication. You can also add more complex logic to the InvoiceGenerator create() method to decide what type of object you want to return. For example, you might want to return an Invoice object if the total price is > 0 , but a CreditNote object if the price is < 0 . Of course, they must extend the same interface (e.g. InvoiceInterface ).

Also, it looks like you are delegating the actual initialization of the Invoice object to the object itself, since the create() method of InvoiceGenerator does nothing more than calling the create() method of the Invoice object, where the actual logic seems to be taking place. This violates the principle of the principle of single responsibility , since the Invoice class is now responsible for storing account setup data and data from the array. Instead, the latter should correspond to the factory class.

So, I would create a class as follows:

 class InvoiceFactory { public function create(array $data) { if ($data['total'] >= 0) { $invoice = new Invoice(); } else { $invoice = new CreditNote(); } $invoice->setTotal($data['total']); // set other data using setters as well return $invoice; } } 

As you can see, there is no create() method that is called in the $invoice object.

Answer 2

If you want to follow SOLID , then the first option is closest to the requirements.

The first option is the InvoiceGenerator class with one responsibility for creating an invoice and does not instantiate the invoice object. An invoice object can be replaced (by extension) with a subclass or a class that implements the invoice interface.

The second option , the invoice object is hard-coded and not open for expansion. It is open for modification if a change in the class of accounts changes in the future. In addition, it is not possible to validate (PHPUnit) InvoiceGenerator using a mock instance of the invoice class.

In my opinion, choose what suits your application and think about who will use and maintain your code.

The ability to make the InvoiceGenerator class less related, and you do not need to instantiate the invoice object in the client code, but you give the opportunity for the client code to set its own instance of the invoice object.

 class InvoiceGenerator { function create(InvoiceInterface $invoice = null) { if (null === $invoice) { $invoice = new Invoice(); } $invoice->create(); } } 

Answer 3

This is actually a complex design problem, given by php as a language.

I prefer less combination than encapsulation (DRY is not repeated).

For myself, I created the software for accounts, this is usually how I structured it.

Inputs: Hours of operation (hourly) Items sold (items) Included in the InvoiceGenerator, which issues an invoice.

 class Hourly { } class Items { } Class InvoiceGenerator { //Returns Invoice //A list of hourly objects and items function createInvoice($state, $hourly = array(), $items = array() { $invoice = new Invoice($hourly, $items); $invoice->applyTaxes($state): //Do Other Things return $invoice; } } 

The key points in all of my projects are # 1 Readabilioty and # 2 Non-Repeating Encapsulating your data, you make it more difficult to test and less flexible.

Array data can be thrown into PHP as an object, but you cannot use other functions for the data. For example, if 1 object is TAX Exempt. Now you should add the ability to test this to your massive array structure.

Or with a less related approach, you have a method that returns a tax value (0 or otherwise).

Hope this example shows why a less connected approach is often the best.

Answer 4

You are considering a problem with a Factory method design pattern , for example:

 class InvoiceFactory { static function create($invoiceData) { $invoice = new Invoice(); $invoice->create($invoiceData); return $invoice; } }