GeoIP2-php icon indicating copy to clipboard operation
GeoIP2-php copied to clipboard

The Dependency Inversion Principle

Open peter-gribanov opened this issue 6 years ago • 6 comments
trafficstars

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

peter-gribanov avatar Jan 09 '19 14:01 peter-gribanov

@oschwald what do you think about this feature?

peter-gribanov avatar Dec 19 '19 15:12 peter-gribanov

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

fmata avatar Oct 27 '22 14:10 fmata

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.

oschwald avatar Oct 27 '22 19:10 oschwald

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.

fmata avatar Oct 31 '22 10:10 fmata

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.

oschwald avatar Nov 10 '22 23:11 oschwald

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.

fmata avatar Nov 11 '22 12:11 fmata