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

On 14 February 2017 at 05:52, Stefan A. <acid24@gmail.com> 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 <david.h@highgearmedia.com > > 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.