Re: [PHP] How do I handle covariant parameters and not fall foul of LSP.

On 14 February 2017 at 05:52, Stefan A. <[email protected]> wrote:

> I think LSP becomes an issue in situations where you want to swap
> implementations. Since I can't really think of such a use case when using
> repositories, you can even name your methods more specifically.
>
> abstract class BaseRepo
> {
>     protected function processEntity(BaseEntity $anEntity)
>     {
>         ...
>     }
> }
>
> class PersonRepo extends BaseRepo
> {
>     public function processPerson(PersonEntity $aPerson)
>     {
>         $this->processEntity($aPerson);
>     }
> }
>
> class CatRepo extends BaseRepo
> {
>     public function processCat(CatEntity $aCat)
>     {
>         $this->processEntity($aCat);
>     }
> }
>
> If the process method is doing storage related things I don't think is a
> good idea to move it to the Entity hierarchy. The main idea of repositories
> is to provide a collection like interface to storage, the other idea common
> to persistence related patterns is to free entities from persistence
> responsibilities and make them more focused on the domain logic.
>
>
>
> On Mon, Feb 13, 2017 at 8:55 PM, David Harkness <[email protected]
> > wrote:
>
>> The problem with LSP is that every BaseRepo must be able to be substituted
>> for any other, but each concrete form only handles its own concrete entity
>> type. The contract of processEntity(BaseEntity) is "I will process any
>> concrete BaseEntity subclass", but PersonRepo violates this contract by
>> declaring that it will only process PersonEntity.
>>
>> Java uses Generics to solve this, but you'll have to fake it with
>> instanceof for safety. Note that you're *still* violating LSP here, but
>> you
>> can't really avoid it given what you want to do.
>>
>>     class PersonRepo extends BaseRepo
>>     {
>>         public function processEntity(BaseEntity $entity)
>>         {
>>             parent::processEntity($entity);
>>             if (!($entity instanceof Person) {
>>                 throw new RepoException('Must be a Person');
>>             }
>>             ...
>>         }
>>     }
>>
>> Well, you could invert the responsibility to save LSP by moving the
>> concrete functionality into the entity, but it will probably be a little
>> hacky.
>>
>>     abstract class BaseEntity {
>>         public abstract function process();
>>     }
>>
>>     class PersonRepo extends BaseRepo
>>     {
>>         public function processEntity(BaseEntity $entity)
>>         {
>>             parent::processEntity($entity);
>>             $entity->process();
>>             ...
>>         }
>>     }
>>
>> Cheers!
>> David
>>
>
>

I'm trying to move to generators for the entities and repos, so I think
having :

protected BaseRepo::persistEntity(BaseEntity $entity): Entity {
// Store the entity based upon required elements from the concrete repo
(table name, primary key, that sort of thing).
// Get a clean copy of the
}

and then have the generator create (sort of thing - without guards, etc.) :

PersonRepo::persist(Person $person):Person{ return
parent::processEntity($person);}
PersonRepo::find(int $personID):Person{ return new
Person(parent::findByID($personID));}

will be the way to go. Every public/concrete repo has a persist() and
find() method suitable for the entity type which proxies to the BaseRepo.

I think the next issue is that I want to enforce the persist() and find()
methods.

I suppose if I'm using a generator, then the concrete classes will be
black-box and as much boilerplate as needed, so ... maybe all moot really.

Thanks for confirming I wasn't going mad with LSP. I still can't see why an
extended BaseEntity is still not a BaseEntity, just a more specialised
variant.

If anyone has some good examples (real world ideally) where LSP is sane AND
includes inheritance (they seem to be where the conflict lies) then I may
be able to learn why I'm doing it wrong.

Regards,

Richard.