On Fri, 11 Mar 2011 00:09:28 +0100
Bernhard Schussek <[email protected]> wrote:
> Hi Benjamin,
>
> Thanks for sharing your thoughts. I will try to answer your questions below.
>
> > 1. FormFactory::getInstance() scares me. Why is there a while() loop, what
> > is a $hierachy? What are all the config objects that are being created?
>
> A field type can inherit from other field types. For example,
> EntityFieldConfig inherits from ChoiceFieldConfig, which inherits from
> FormConfig, which inherits from FieldConfig. The inheritance is done
> dynamically (by calling the getParent() method on the config), because
> this flexibility is needed.
>
> In the loop, all config objects in that hierarchy are traversed. I.e.,
> if you create an entity field, FieldConfig, FormConfig,
> ChoiceFieldConfig and EntityFieldConfig are traversed in that order
> (parent to child). The field instance is passed to each config
> instance for configuration.
>
> The config objects are like factories, only more dynamic. There is
> exactly one config object per field type, which is loaded lazily from
> the DIC.
Ah ok, one config per field type explains the need for FieldPlugins. This
brought me to look at config loaders. I don't think they are necessary at all.
You should just define a service container tag "form.config" and have all of
them injected into a FormFactory and defined as private services.
That way you could have a "DefaultFormFactory" that can be used outside the
Framework with all the default configs hardcoded in them, and a
DoctrineFormFactory that extends it and contains the EntityManager reference.
>
> > 2. There is no way to build a form programatically anymore, everything has
> > to go through the FormFactory. This makes using forms MUCH more complicated.
>
> That's not true. You _can_ build forms programatically, but it's
> complicated because you have to know all the dependencies. The
> previous approach, where fields/forms created their dependencies
> themselves (and thus were easy to instantiate without factories) was
> flawed.
First, the FormFactory dependency inside Form sort of hinders me from doing it
programatically. This is linked to point 4.
Second, then I still like to have a builder object that has convenience create
methods for all the default fields.
For simple forms i'd still like to build them programatically, maybe something
like:
$form = new Form('contact', new DefaultFormFactory(), $csrfService,
$validatorService); // see point 1 for DefaultFormFactory.
$form->setData(new ContactRequest());
$form->add('text', 'email');
$form->add('textarea', 'message'));
The convenience compared to having to define an extra class, registering it in
the DI and so on is just undeniable :-)
Simpler would therefore be:
$builder = new FormBuilder($csrfService, $validatorService);
$form = $builder->createForm('contact');
$form->setData(new ContactRequest());
$form->add('text', 'email');
$form->add('textarea', 'message'));
You could even register the form builder class in the DI if you give it a third
optional formfactory parameter. This would allow both programmatic and
AbstractConfig extending configurations.
>
> > 3. Tons of new object instances are needed, renderers, configs, plugins.
> > None are loaded lazily. What are the performance implications for
> > medium/large forms?
>
> Configs are loaded lazily. All objects in the fields/forms are used in
> most requests that instantiate forms, except for the renderer, which
> is not used if the form is only bound, but not rendered (e.g. on
> successful submission). I can't answer the performance question,
> because I haven't done any benchmarks yet.
>
> > 4. Why is the FormFactory not a builder object around the lightweight
> > objects that existed before? Now a FormFactory has to be injected into
> > every form. This feels wrong. It seems the factory is only used inside
> > Form:add(). Wouldn't it be better to add a builder like:
>
> I thought about this already, but then the builder would have to
> replicate the whole interface of Form, which isn't very nice either.
>
> > Also this way the ugly code from Form:add using func_get_args could be
> > removed.
>
> The func_get_args code will be removed by changing the signatureof add() to
>
> function add($property, $field = null, array $options = array())
How would that work? Isnt property a required input for field constructor also?
>
> > 5. How can i create field elements outside of the config/factory stuff?
> > Instead of the FormFactory a builder object would be very useful.
>
> By creating new Field() and passing all dependencies that you find in
> FieldConfig manually.
See point 2. FormBuilder
>
> > 6. What is a field plugin? Why does every field need one, isnt the config
> > already there to split separation of field and configuration?
>
> FieldPlugin is a renderer plugin. Renderer plugins dynamically set
> view variables that will be available in the template of a field.
> Because FieldPlugin depends on its field, we need a separate instance
> for each field.
>
> > 7. Why is the EventManager used inside Field? Why is there one EventManager
> > and one Filterchain per Field? This kind of object inflation is what makes
> > Zend Form so slow. How about creating them lazily in addEventListener() and
> > addFilter() and checking for this in bind() and submit()?
>
> Once the EventDispatcher has been changed, there will be only one
> EventDispatcher per field (no Filterchain anymore). The reason is that
> each field has different listeners for filtering the values or
> transforming the field, so we need to have different dispatchers.
>
> Lazy loading here is a good idea.
If by lazy loading i get rid of them being used in all but the few fields that
need events its ok.
>
> > 8. The $form->getRenderer() seems weird to me. Can't it be hidden inside
> > the twig and php template helpers?
>
> There are no helpers anymore. In the template, you deal with the
> renderer object. Hiding this sounds like magic to me.
>
> If you have any more improvement suggestions (I prefer pull requests)
> I'm more than happy to hear them! Please, keep them coming.
Why does it sound like magic to you? Its completely obvious to me.
Say you have $templating->render("..", array("form" => $form)); and "{{
form_errors(form) }}".
Why not have the template method call $form->getRenderer()?
That we the whole ThemeInterface stuff can be dropped, i can't make a sense out
of it. This was previously defined in the template where it belonged, it was
much cleaner. Just because renderes do exist we don't have to change the way
how the twig / php rendering worked as before, just add the additional
$field->getRenderer() calls everywhere inside the helpers.
greetings,
Benjamin
>
> Bernhard
>
> --
> 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
>
--
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
