veguv veguv - 25 days ago 6
PHP Question

When passing classes by the service and when use type hinting

Which version of my example code is better and correct object-oriented?

1)

class ServiceA
{
private $serviceB;

public function __construct(ServiceB $serviceB)
{
$this->serviceB = $serviceB;
}

public function first() : int
{
return 1 + $this->serviceB->second();
}
}

class ServiceB
{
public function second() : int
{
return 2;
}
}


2)

class ServiceA
{
private $serviceB;

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

public function first() : int
{
return 1 + $this->serviceB->second();
}
}

class ServiceB
{
public function second() : int
{
return 2;
}
}


This is without type-hinting in constructor, because if the class has to be versatile and easy to change should not be there type-hinting.

3)

class ServiceA
{
private $serviceB;

public function __construct()
{
$this->serviceB = new ServiceB();
}

public function first() : int
{
return 1 + $this->serviceB->second();
}
}

class ServiceB
{
public function second() : int
{
return 2;
}
}


Because I want to always use ServiceB in this class. But does not that break the law of Demeter?

4)

class ServiceA
{
private $serviceB;

public function first() : int
{
$this->serviceB = new ServiceB();

return 1 + $this->serviceB->second();
}
}

class ServiceB
{
public function second() : int
{
return 2;
}
}


This is similar to 3. I use ServiceB only there where I want it.

5)

class ServiceA
{
public function first() : int
{
$serviceB = new ServiceB();

return 1 + $serviceB->second();
}
}

class ServiceB
{
public function second() : int
{
return 2;
}
}


The simplest version.

In all of the examples, I mean that in ServiceA I always have to use ServiceB. If I want to change ServiceB to ServiceC then I will also have to change the contents of ServiceA class, so in that case I need to use example 1?

I have separated these classes to preserve the principle of single responsibility and they always have to work together. In addition, ServiceB I use elsewhere.

Answer Source

Your code needs to tell exactly what it's doing to avoid confusion and misinterpretation. So it should version be 5).

  1. This example is not good because it reads as "A can be configured with an instance of B which is created and configured somewhere else".

  2. Even worse than 1) because it doesn't tell us anything about the constructor parameter. If you want more flexibility you should use an interface.

  3. Reads as "A always needs B and might use it in several places".

  4. Just an example of a bad code. If you want B to be lazily initialized in A you should put into factory method which will tell your intentions clearly.

  5. Reads as "Method first of A needs B" which is exactly what you wanted.

Another possible solution if B is heavy to initialize is to initialize it lazily and store in a property:

/** 
 * @var B 
 */
private $_b

private function getB() : B
{
    if (!$this->_b) {
        $this->_b = new B();
    }

    return $this->_b;
}

Also, if A always needs B you might have a look at aggregates from domain-driven design. But it's hard to advise something good here because we don't know much about your project.

My general advice is to write the simplest possible code when you are unsure about your abstractions or relations between different objects. In your question 5) can be easily refactored into 1) or 3) if needed. So at this point there is no need to waste your time on such question. Look at your code as at something which evolves with time. You don't need to provide final solutions with each piece of code you write.