The Dependency Inversion Principle

[peter-gribanov]

I add a Reader service interface to add the ability to invert dependencies.

[peter-gribanov]

@oschwald what do you think about this feature?

[fmata]

@oschwald may you merge & release this please ? It's impossible to test our implementation using Reader without interface fully compliant.

[oschwald]

Is there a reason why you cannot just mock the reader via createMock (or, I suppose, subclass it) for testing? I am somewhat reluctant about this change as it makes adding a new method to the reader a breaking change unless we let the interface fall out of sync with the class.

[fmata]

Hi @oschwald, my code depends on Reader I have no reason to subsclass it to make a wrapper doing nothing. Althouth this can be useful for testing purpose, I think it's more reliable to depends on vendor's interface rather than a layer in my code.

I you are septical about BC, maybe can you release a new major 3.0 with this interface. In all cases, the BC is easy to address.

[oschwald]

My concern about breaking changes is that every new method added to the interface is a breaking change. This would cause us to have many more major releases, which often make people hesitant to upgrade. I suppose one "solution" would be to say the interface is not covered by our semver versioning, but that would likely lead to confusion as well.

[fmata]

When you add a new method, you can always create a new interface that extends ReaderInterface and merge the two in the next major version.

Ex :

Now in 2.x

interface ReaderInterface
{
    public function city($ipAddress);
    public function country($ipAddress);
    public function anonymousIp($ipAddress);
    public function asn($ipAddress);
    public function connectionType($ipAddress);
    public function domain($ipAddress);
    public function enterprise($ipAddress);
    public function isp($ipAddress);
    public function metadata();
    public function close();
}

When you add a new method in 2.y you can decorates Reader and introduce a new interface

interface SuperInfoInterface extends ReaderInterface
{
    public function mySuperInfo($ipAddress);
}

class SuperInfoReader implements SuperInfoInterface
{
    private $decorated;

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

    public function mySuperInfo($ipAddress)
    {
        ...
    }
    
    public function city($ipAddress)
    {
        return $this->decorated->city($ipAddress);
    }

    ...
}

In 3.0, you will merge SuperInfoInterface in ReaderInterface and voilà.
You can as well introduce a new Reader class by now to address BC.