Abstracting related functionality using interfaces vs tight coupling

Question

I am trying to understand what really defines tight coupling. I have read a number of posts on the subject but one thing still doesn't sit right with me.

I understand that classes should be injected into other classes using their interfaces rather their concrete implementations. I also understand that if a class adheres to an interface then any class which uses the injected interface can call the public functions defined in the interface and expect similar functionality.

interface iFormatter()
{
    public function format(array $order): array
}

public class OrderFormatter implements iFormatter
{
    public function format(array $order): array
    {
        // ...

        return $formattedArray;
    }
}

public class OrderGenerator implements iGenerator
{
     private $formatter;

     public function __construct(iFormatter $formatter) 
     {
         $this->formatter = $formatter;
     }

     public function generate()
     {
          // ...

          return $this->formatter->format($order);
     }
}

So I think in the case where only the formatter changes, this would be defined as loosely coupled;

$example = new OrderGenerator(new CarOrderFormatter);
$example->generate();

$example = new OrderGenerator(new VanOrderFormatter);
$example->generate();

What I don't quite get though is when you abstract responsibilities away from each other but those classes are still closely tied to each other. Something like

$example = new CarOrderGenerator(new CarOrderFormatter);
$example->generate();

$example = new VanOrderGenerator(new VanOrderFormatter);
$example->generate();

Yes you could pass in a different formatter to these generators, but surely more often that not some error would occur within the format function if it is expecting certain data from the generate function in the XXXOrderGenerator concrete class?

So I believe I have abstracted the responsibilities to their own classes in the last example above and although interfaces have been used i'm not sure if this is still tightly coupled or is technically loosely coupled? Or if I have missed the point entirely...

Cloutman

Answer 1

Classes are said to be tightly coupled when one class has a dependency on another or relies on the internals or specifics of the other.

In the case of the first example you gave OrderGenerator and OrderFormatter are not tightly coupled because neither one is dependent on the other: they both depend on the iFormatter. Another way of putting it is OrderFormatter knows nothing about OrderGenerator and OrderGenerator knows nothing about OrderFormatter, it just knows that the object that is passed to it implements the function 'format'. You have highlighted this when you pass either CarOrderFormatter or VanOrderFormatter to OrderGenerator without adverse result.

In the case of the second example where CarOrderFormatter is passed to CarOrderGenerator you mentioned that surely errors would occur of you passed say VanOrderFormatter to CarOrderGenerator. If the code in CarOrderGenerator is relying on the implementation details of CarOrderFormatter then the classes are tightly coupled even though the iFormatter interface is what the CarOrderGenerator sees. This code would be confusing because CarOrderGenerator is saying with its 'contract' that it doesn't care what formatter is passed but clearly it does. It would therefore be better in terms of the clarity of the code if the CarOrderGenerator explicitly said that you could only pass a CarOrderFormatter to its constructor. The abstraction (that is, the use of the interface) is not a bad thing however and thought needs to be put into how exactly the interface should be defined. Say for example instead of having just an iFormatter interface instead you had iCarOrderFormatter and iVanOrderFormatter both of which are defined the same but the CarOrderGenerator required iCarOrderFormatter and the VanOrderGenerator required iVanOrderFormatter. The your example could become:

$example = new CarOrderGenerator(new CarOrderFormatter
$example->generate();
$example = new CarOrderGenerator(new SportsCarOrderFormatter)
$example->generate();

or

$example = new VanOrderGenerator(new PassengerVanOrderFormatter
$example->generate();
$example = new VanOrderGenerator(new CargoVanOrderFormatter)
$example->generate();

The key thing to realize is that the interface is introduced to create flexibility in what can be passed to the generator without it causing problems.

Exosphere

Answer 2

You're correct that if you run into car/van concretes for iGenerator and iFormatter you're probably going to have a bad time mixing the dependencies. But the thing is you should not run in to that situation, if you do you should go back to the drawing board because you made a wrong turn.

I don't know if your example is contrived or based closely (or not) to your problem domain. In either case the issue is a poor design. Your interface should represent a single responsibility or purpose. To this end what is the purpose of iOrderGenerator and iFormatter? Your example all iOrderGenerator does is wrap iFormatter. I could refactor iOrderGenerator out of the design because it serves no purpose. Problem. Solved.

Okay let's not take the easy way out and say iOrderGenerator did have a purpose. Well that purpose should be different from iFormatter. If your implementation of the interface is stepping over the boundaries of its single purpose or replicates the purpose of another interface you likely have a bad design. So as an example let's say the purpose of iFormatter is to print orders differently based on type of order (van or car). Then iOrderGenerator is responsible for creating orders (of any type). But I may have more than one way to create an order. CsvOrderGenerator or XmlOrderGenerator or BulkOrderGenerator (but never CarOrderGen or VanOrderGen ).

Nit picking on your example we can point out design problems that explain how you ended up with an uncomfortable situation when you're trying to demo a good code concept.

You're iFormatter is a bad interface because it returns a vague value (array). Anything that uses the return value would need intimate knowledge of how the concrete worked to know what items were or weren't contained in the array. Ie how would I know there was an orderId key value without reading the code or viewing the result (debug/print). Instead iFormatter should return a fixed well described type ie CarFormat or VanFormat each of which might implement an iFormat that provides common accessors for order number and price.

iOrderGenerator should probably generate an iOrder. Which might have a method getFormatter : iFormatter.

Those changes might clean up the design and indicate clearer purposes for each concrete that won't lead you to your initial what if.

Superannuate

Thank you for your point about vague return values, that has changed my view on how to structure classes entirely. I felt you deserved a bounty for your explanation too. – Cloutman