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?
5 Answers 5
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– Aitch2016年01月02日 21:33:21 +00:00Commented Jan 2, 2016 at 21:33
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.
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.
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.
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.
Explore related questions
See similar questions with these tags.
Item
specification, and check that parameter has the typeMovie
.