4

I am trying to build a PHP application using the Repository Pattern but I'm not sure how I should implement the save method.

I have an abstract class called ItemRepository which have the following method:

abstract class ItemRepository
{
    public function save(Item $item);
}

Where Item also is an abstract class.

Now I want to implement the class MovieRepository which extends ItemRepository. Here I want to save instances of the Movie class which extends Item.

Though doing it like this in PHP

// MovieRepository.php
class MovieRepository extends ItemRepository
{
    public function save(Movie $movie)
    {
        ...        
    }
}

gives the following error

Declaration of MovieRepository::save() should be compatible with ItemRepository::save(Item $item)


What is the right way to do this?

  • As I can see, this is a limitation of the PHP. In C++/C# you could use tempates/generics. In PHP you can drop in type specification, or you can keep Item specification, and check that parameter has the type Movie. – Mark Shevchenko Sep 15 '15 at 7:02
  • The repository should just translate the save() to a lower laying library. for instance a dbal. Look into Doctrine and how doctrine handles this. – Pinoniq Sep 18 '15 at 11:35
1

Drop the Item/Movie types from the save() method:

abstract class ItemRepository
{
    public abstract function save($item);
}

class MovieRepository extends ItemRepository
{
    public function save($movie)
    {
        ...        
    }
}

PHP is primarily dynamically typed language and while I understand the desire to use types as much as possible, in this case PHP's type system is not strong enough to express what you need. In other languages (Java, C++), you could express it with generics, which PHP lacks.

  • this should be the correct answer! OP does not want anybody to tell him something about the pattern. It's just a PHP issue: drop the types. – Aitch Jan 2 '16 at 21:33
0

You can't. A subclass should support the same methods as the parent class, without imposing any additional restrictions. That way if you encounter a variable of type ItemRepository, you don't need to know which subclass it is; you can always call save and pass it an Item.

If you don't want this, you can add a different method, like saveMovie(Movie $movie). But you may want to reconsider whether you want a subclass at all.

0

Unless the ItemRepository and MovieRepository classes are actually related, then you don't need a parent class. Instead, go with MovieRepository implementing an interface:

interface IMovieRepository
{
    function save(Movie $movie);
}

class MovieRepository implements IMovieRepository
{
    public function save(Movie $movie) {
        // ...
    }
}

Any place you need the movie repository, use the interface instead. A sample controller utilizing constructor injection is below:

class MoviesController
{
    private $movies;

    public __construct(IMovieRepository $movies = null) {
        $this->movies = $movies || new MovieRepository();
    }

    public function create($params) {
        $movie = new Movie();
        // Populate $movie based on $params

        $this->movies->save($movie);
    }
}

If anything, I would create a BaseRepository class that does not implement any CRUD methods, and just contains common functionality for all repository classes. The saving of one entity to another will change, therefore they are not related and should not share a common parent class.

0

You might benefit from studying Doctrine2. While it also uses the "Repository Pattern", it also uses the "Data Mapper Pattern".

The following is an example of using the entity manager:

/* $movieRepository = MovieRepository -> extends EntityRepository -> implements ObjectRepository */
if (null !== $movie = $movieRepository->findOneBy(array('name' => 'Mad Max'))) {
    $movie = new Movie();
    $movie->setName('Mad Max');

    /* $entityManager = EntityManager -> implements ObjectManager */
    $entityManager->persist($movie);
}

$movie->setViews($movie->getViews() + 1);

$entityManager->flush();

Notice here that Doctrine2 defines two major classes (one abstract).

abstract class EntityRepository implements ObjectRepository
{
    ...
}

class EntityManager implements ObjectManager
{
    ...
}

Traditionally the repository is only used to make queries, i.e. for selections, rarely updates or inserts. ObjectRepository interface defines common select methods:

  • find($id)
  • findBy(array $criteria)
  • findOneBy(array $criteria)

The entity manager however, has the methods pertaining to persistance:

  • persist($entity)
  • flush($entity = null)

The entity manager never requires a specific type of entity besides the "range" of classes registered as being mapped to it's database.

If you want to create your own persistence library, you could use the Doctrine\Common\Persistance interfaces to help you design it.

  • ObjectRepository
  • ObjectManager

A reason you might choose to persist entities and flush all at once, is to create a coherent transaction which "finalizes" the process. Opposed to "saving" each entity individually during the process, which would gradually build up the amount of IO in your application, effecting performance.

TL;DR

Incorporate the Data Mapper Pattern in your design, and migrate the persistence away from your repositories, and instead encapsulate it into a single manager for your database. Then limit the repository to selection queries.

0

I'm going to guess, that what you're trying to do, is have a common implementation of save() but exposed with the correct type-check for each repository type? (You didn't declare save() as abstract in your ItemRepository, so I'm guessing you had a method body in there?)

Since the argument-type for save() in different repository types are going to differ, we're not talking about the same save() method - in other words, naming the method save() in each repository type is going to be a convention, but they are different methods, since e.g. save(Movie $movie) is incompatible with save(Item $item), because Movie is an Item, but an Item is not necessarily a Movie.

Based on that, I would suggest extracting an internal save-method into an abstract base-class.

abstract class AbstractItemRepository {
    protected function saveItem(Item $item) {
        // ...
    }
}

class ItemRepository extends AbstractItemRepository {
    public function save(Item $item) {
        $this->saveItem($item);
    }
}

class MovieRepository extends AbstractItemRepository {
    public function save(Movie $movie) {
        $this->saveItem($movie);
    }
}

In other words, put shared functionality in an abstract base class - but leave the public interface definition up to the individual implementation.

That said, please see answer by @GregBurghardt who points out:

Unless the ItemRepository and MovieRepository classes are actually related, then you don't need a parent class

In other words, if the save functionality isn't a common implementation, there's no point in trying to abstract this, beyond maybe trying to enforce a method naming convention; but that's not really what abstraction is for.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.