Skip to content

Commit

Permalink
Moved doctrine knowledge into a manager class
Browse files Browse the repository at this point in the history
The controller didn't need to know about the implementation
details of doctrine, it only needs to know that our application
can provide some blog posts objects. Therefore we introduce
a "Manager" class whose job it is to do that.
  • Loading branch information
peterjmit committed Sep 4, 2013
1 parent 55c0bf8 commit 4a87e1d
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 20 deletions.
17 changes: 5 additions & 12 deletions spec/Peterjmit/BlogBundle/Controller/BlogControllerSpec.php
Expand Up @@ -5,25 +5,18 @@
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\Common\Persistence\ObjectManager;
use Doctrine\Common\Persistence\ObjectRepository;
use Peterjmit\BlogBundle\Model\BlogManagerInterface;

use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;
use Symfony\Component\HttpFoundation\Response;

class BlogControllerSpec extends ObjectBehavior
{
function let(
ManagerRegistry $registry,
ObjectManager $manager,
ObjectRepository $repository,
BlogManagerInterface $manager,
EngineInterface $templating
) {
$registry->getManager()->willReturn($manager);
$manager->getRepository('PeterjmitBlogBundle:Blog')->willReturn($repository);

$this->beConstructedWith($registry, $templating);
$this->beConstructedWith($manager, $templating);
}

function it_is_initializable()
Expand All @@ -32,11 +25,11 @@ function it_is_initializable()
}

function it_should_respond_to_index_action(
ObjectRepository $repository,
BlogManagerInterface $manager,
EngineInterface $templating,
Response $mockResponse
) {
$repository->findAll()->willReturn(array());
$manager->findAll()->willReturn(array());

$templating
->renderResponse(
Expand Down
14 changes: 6 additions & 8 deletions src/Peterjmit/BlogBundle/Controller/BlogController.php
Expand Up @@ -2,24 +2,23 @@

namespace Peterjmit\BlogBundle\Controller;

use Doctrine\Common\Persistence\ManagerRegistry;
use Peterjmit\BlogBundle\Model\BlogManagerInterface;
use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;

class BlogController
{
private $doctrine;
private $manager;
private $templating;

public function __construct(ManagerRegistry $doctrine, EngineInterface $templating)
public function __construct(BlogManagerInterface $manager, EngineInterface $templating)
{
$this->doctrine = $doctrine;
$this->manager = $manager;
$this->templating = $templating;
}

public function indexAction()
{
$entityManager = $this->doctrine->getManager();
$posts = $entityManager->getRepository('PeterjmitBlogBundle:Blog')->findAll();
$posts = $this->manager->findAll();

return $this->templating->renderResponse('PeterjmitBlogBundle:Blog:index.html.twig', array(
'posts' => $posts
Expand All @@ -28,8 +27,7 @@ public function indexAction()

public function showAction($id)
{
$entityManager = $this->doctrine->getManager();
$post = $entityManager->getRepository('PeterjmitBlogBundle:Blog')->find($id);
$post = $this->manager->find($id);

if (!$post) {
throw $this->createNotFoundException(sprintf('Blog post %s was not found', $id));
Expand Down
9 changes: 9 additions & 0 deletions src/Peterjmit/BlogBundle/Model/BlogManagerInterface.php
@@ -0,0 +1,9 @@
<?php

namespace Peterjmit\BlogBundle\Model;

interface BlogManagerInterface
{
function findAll();
function find($id);
}

12 comments on commit 4a87e1d

@docteurklein
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's very good stuff! Great that you put that into words, in a clear and concrete example!

However, I would have implemented the interface in this commit, so that you can have a working controller at the end :)

I know that following DDD would make you create a semantic interface, but https://github.com/doctrine/common/blob/master/lib/Doctrine/Common/Persistence/ObjectRepository.php is already a good fit.

What about making the repository a service ?

@everzet
Copy link

@everzet everzet commented on 4a87e1d Sep 5, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even more specifically - use concrete BlogRepository.

@docteurklein
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right.

@peterjmit
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you chaps are right, the example would be simpler/would benefit from just injecting the repository rather than BlogManagerInterface.

Its more habit rather than anything because I've always found the way FOSUserBundle had managers for the entities with the repository injected useful.

@everzet
Copy link

@everzet everzet commented on 4a87e1d Sep 5, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterjmit just don't forget that interfaces though are cleanest, are not the only way to define public communication protocol. Classes public API forms communication protocols as well. And yes, interfaces are the purest form of protocol. Question actually is do you need this purest protocol in that particular case ;)

@silviuvoicu
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterjmit how to inject BlogManagerInterface in the constructor of BlogController, for the templating is easy, because is already a service with an id.

@peterjmit
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@silviuvoicu I'll do a follow up with some concrete code, but you would need to both write a concrete implementation of BlogManager define it as a service, define your controller as a service then pass the manager service as an argument

@silviuvoicu
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterjmit Thanks.

@cordoval
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess when you say "Classes public API " you talking about adjusting rather the uniquitous language (DDD related) so changing the method names on our models, that is defining a domain language I guess ...? or what exactly if i am totally lost 👶 ?

@docteurklein
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, he means the public methods defined for a class. They are, by definition, accessible, and "open to receive messages". Otherwise, they would be protected.

@cordoval
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the prescription Dr. 👶

@cordoval
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.