On 3/20/10 12:14 PM, Nicolas Perriault wrote:
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
}

the first is already possible:

$this->container['user'] is equivalent to $this->container->getUserService()


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?

Almost no impact. This is already what we use... and the performances are quite good.


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(),
));

But you loose a lot here:

 * what is you have a typo in the key ('usre')?
 * not possible anymore to typehint
 * no default value

### 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;
}

Same comment as above:

* not explicit anymore (relies on too much conventions) - you need to have a look at the body of the method to know what to pass as a parameter or service
 * what is you have a typo in some keys ('usre')?
 * not possible anymore to typehint
 * not possible anymore to have default values


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.

'lot of naming conflitcs expected': I'm not sure it would happen that much. I had a look at a lot of my own controllers in symfony 1 and the number of needed arguments is mot of the time (read always) very low.


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?

hehe, it means that your controller is a real mess ;)

Fabien


These should definitely be categorized imho.

Just my two cents.

++


--
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.

Reply via email to