On Sat, Mar 20, 2010 at 11:32 AM, Fabien Potencier
<[email protected]> wrote:
> This RFC discusses the possible enhancements to the current controller
> implementation in Symfony 2. Feel free to give your opinion.
Okay I'll just be thinking out loud, expect some inaccuracies.
> That's easy enough and means that you can use any service in your actions:
>
> function indexAction()
> {
> $this->container->getUserService()->setAttribute(...);
> }
Makes me think it's a bit verbose, could be replaced by something like:
function indexAction()
{
$this->services['user']->setAttribute(...);
// or even shorter:
$this->__user__->setAttribute(...); // <-- __x__ convention could be
replaced, of course
}
> Also, when executing the action, the method arguments are injected by
> matching
> their names with the routing variables thanks to introspection:
>
> function showAction($id)
> {
> }
What's the impact on performances by the reflection? Is it done at
runtime or can the work be efficiently cached?
> * The controller is tied with the container which is not a good idea
> (separation of concerns).
True.
> * Using a service in an action is quite verbose
> ($this->container->getUserService())
See my previous comment...
> * To mitigate the previous problem, we provide extensions to the default
> Controller, like DoctrineController, but of course, it's not possible to
> inherit from several base classes. So, it's only a partial solution.
Inheritance is hell. I'd rather use clever composition, always.
> How to test a controller?
> The main problem is that testing the controller involves a container, which
> is not a very clean solution.
Yeah, don't make the container the new sfContext, por favor ;)
> Possible changes for PR2
> ------------------------
> ### Inject the needed services in the constructor
>
> As for the action, we can inject the services in the constructor by using
> introspection:
>
> protected $user, $doctrineManager;
>
> Problem: The constructor need all services needed by all methods, which is
> not a good idea (we will create too many services most of the time). And then,
> it's up to the developer to store the services in protected variables. That
> means some boilerplate code.
Agreed, I don't like much the protected variables trick; I'd rather
use a dedicated one:
protected $__requirements__ = array('user', 'doctrineManager');
> Testing a controller is much cleaner than before though:
>
> $user = new UserMock();
> $controller = new ArticleController($user);
Could then be:
$controller = new ArticleController(array(
'user' => new UserMock(),
));
> ### Inject services as protected properties
> To avoid the boilerplate code, we can inject the services directly by
> introspecting the protected variables
> The biggest problem here is the magic involved. And as the controller can
> have
> non-service protected variables, we won't be able to thrown any exception if
> the service does not exist:
>
> protected $user;
> protected $pi = 3.14;
> protected $someNonServiceVariable;
Nightmares expected, don't do this.
> ### Inject services in each controller
> As we already do some introspection to pass arguments to the action, we can
> also inject services:
>
> function showAction($id, $user, $doctrineManager, $pi = 3.14)
> {
> }
Well, in such scenatio, couldn't we have:
function showAction($parameters, $services)
{
$id = $parameters['id'];
$user = $services['user'];
$pi = $this->pi;
}
Of course the $services parameters here is not a Container instance ;)
just a simple hash of services instance, eg:
array (
'user' => $user,
'doctrineManager' => $doctrineManager,
// ...
)
> The action code is now very concise:
>
> function showAction($id, $user, $doctrineManager)
> {
> $user->setAttribute(...);
> }
My main concern is regarding variable precedence; imagine you got a
'user' request|routing parameter, and a 'user' service, lot of devs
will be confused if getting a string instead of a User instance (or
the opposite). Also, lot of naming conflicts expected.
I'm really not fond of having everything passed as arguments in the
controller method signature. What if I have twelve services and ten
request parameters, plus four protected local variables?
These should definitely be categorized imho.
Just my two cents.
++
--
Nicolas Perriault
http://prendreuncafe.com - http://symfonians.net
Mobile: +33 660 92 08 67
--
If you want to report a vulnerability issue on symfony, please send it to
security at symfony-project.com
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en
To unsubscribe from this group, send email to
symfony-devs+unsubscribegooglegroups.com or reply to this email with the words
"REMOVE ME" as the subject.
