GeoIP2-php
GeoIP2-php copied to clipboard
The Dependency Inversion Principle
@oschwald what do you think about this feature?
@oschwald may you merge & release this please ? It's impossible to test our implementation using Reader without interface fully compliant.
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.
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.
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.
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.