Loose coupling vs Encapsulation. Best approach for a balanced design

Question

According to the next 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 coupling between the InvoiceGenerator and Invoice classes, because InvoiceGenerator does not require the Invoice class. Plus, it could handle not only a class, but a whole interface with very little modification. I've read many times this principle that says: code to interface, not implementation. The downside of this case is that I'm forced to instantiate the Invoice class in the client code.

The second one has more encapsulation. All the process of instantiation and creation of an invoice is delegated to the InvoiceGenerator class. Even though both classes are coupled, this makes sense because an "invoice generator" wouldn't do anything without invoices.

Which would you consider most appropriate? Or what are the key points for a balanced design between both of them?

My opinion is to read the following articles and get your own decision. http://pdepend.org/documentation/software-metrics/coupling-between-objects.html http://www.ibm.com/developerworks/library/os-php-7oohabits/#toggle http://en.wikipedia.org/wiki/Loose_coupling http://typicalprogrammer.com/doing-it-wrong-getters-and-setters/ — , Dec 20 '14 at 12:41

Nic Wortel · Accepted Answer · 2014-12-20T22:41:04.790

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

However, as I said, the InvoiceGenerator class seems to be a factory class, and the whole idea of a factory class is that the logic for instantiating an object is encapsulated within that class (as in your second example). If you would pass the Invoice object as an argument (as in your first example), you would have to instantiate it somewhere else, and if you have multiple places in your code where this happens, you would end up duplicating the instantiating logic. The whole idea of the factory pattern is to prevent this.

By using a factory class you can encapsulate the object instantiation logic and avoid duplication. You can also put some more advanced logic into the InvoiceGenerator's create() method to decide what type of object you want to return. For instance, 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 should both extend the same interface (for instance InvoiceInterface).

Also, it looks like you are delegating the actual initializing 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 take place. This violates the single responsibility principle, because the Invoice class is now both responsible for holding data about an invoice and setting all the data from an array. The latter should instead be the responsibility of the factory class.

So, I would create the class like this:

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 being called on the $invoice object.

This makes more sense to me, since the purpose of the generator is to create invoices. I did know about the factory pattern, but didn't focus the problem with it. What I get, if I'm not wrong, is: if a class goal is to generate other classes, then the factory pattern, which uses encapsulation, fits well. Otherwise if a class depends on another class, or better, if a class depends on a public interface several classes meet, then passing it as an argument is a must. Never instantiate within it. Is this correct? — Luis Martin, Dec 25 '14 at 10:33
Another question: generally speaking, do we always need a factory class? Since classes must end up being instantiated somewhere. — Luis Martin, Dec 25 '14 at 10:50
In reply to your first comment: yes, that is basically correct. If the responsibility of a class is to generate objects, it *is* a factory class. But yes, if a dependent object uses a dependency to perform certain tasks, you should always inject it instead of hard-coding the class name in the dependent class. This keeps your code loosely coupled and more flexible. Therefore, you should always depend on abstractions (interfaces), not on concrete classes - otherwise it would defeat the purpose of dependency injection. — Nic Wortel, Dec 27 '14 at 23:33
In reply to your second comment: no. Because then you would also need a factory to create the factory, etc. I only use factories when the logic to instantiate a certain class is complicated, and I want to encapsulate it. In other cases, I will simply instantiate the classes in the entry point of the application (index.php) and then either use them directly or inject them as dependencies into other objects. In case of Symfony applications / bundles, you can add them to the service configuration so that they become available in Symfony's service container. — Nic Wortel, Dec 27 '14 at 23:44

score 1 · Answer 2 · answered Dec 20 '14 at 21:59

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

The first option, InvoiceGenerator class with one responsibility to create an invoice and does not instantiate an invoice object. The invoice object can be replaced (extension) with a sub-class or a class that implements the invoice interface.

The second option, the invoice object is hardcoded and is not open for extension. It is open for modification if the implementation of the invoice class changed in the future. Also, it is not possible to test (PHPUnit) the InvoiceGenerator with mock instance of the invoice class.

In my opinon, choose what suit your application needs while considering who is going to use and maintain your code.

An option to make the InvoiceGenerator class less coupled and you don't have to instantiate the invoice object in the client code but you give the option 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();
   }
}

Thanks for your opinion. I get what you said. However, after reading Nic's answer, the factory pattern, which encapsulates instantiation, would fit better in this example. Anyway I get the point that it's a bad design when you encapsulate the instantiation of a class the client class depends on. — Luis Martin, Dec 25 '14 at 10:46

score 1 · Answer 3 · answered Dec 24 '14 at 13:50

This is actually a difficult design problem given php as the language.

I do prefer less coupling rather than encapsulation (DRY dont repeat yourself).

For myself, I have built invoice software this is generally how I have it Structured.

Inputs: Hours Worked (Hourly) Items Sold (Items) Goes into a InvoiceGenerator which outputs 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 my projects are #1 Readabilioty and #2 Not Repeating yourself By encapsulating your data, you make it more difficult to test and its less flexible.

Array data can be thrown around in PHP like an object but you miss the ability to have other functions on the data. Like for instance if 1 object is TAX Exempt. You now have to add to your massive array structure the ability to check for that.

Or with the less coupled approach you just have a method that returns the tax value (0 or otherwise).

I hope that example shows why a less coupled approach is often the better one.

You said you prefer less coupling to encapsulation, but your example encapsulates the invoice instantiation within the generator. — Luis Martin, Dec 25 '14 at 10:22

score 0 · Answer 4 · answered Dec 20 '14 at 12:37

You shoud consider approach the problem with a Factory Method Design Pattern something like:

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

Loose coupling vs Encapsulation. Best approach for a balanced design

4 Answers4