-- Nathan <[EMAIL PROTECTED]> wrote
(on Wednesday, 12 March 2008, 03:24 PM -0700):
> Sorry if I what I said came across as being hostile or presumptuous, I
> didn't mean to slight the hard work you've put into Zend_Form.
And apologies on this end if I came across... cross. I'd just discovered
the ViewRenderer bug in the RC2 release, which had me pretty grumpy.
> Your arguments are certainly convincing, I'd just like to mention that this
> was the simple use case that my idea for static defaults came from:
>
> Zend_Form_Element::setDefaultDecorators(array(new
> Zend_Form_Decorator_ViewHelper));
>
> However, I can certainly see how it might be unnecessarily complex and
> confusing to have certain elements override that. I was stuck on
> Zend_Form_Element instead of overloading the defaults from within Zend_Form
> subclasses. Good call.
There's actually another issue I didn't mention: there are also good
reasons for each element having its own decorator instances. Most
decorators can receive options that allow you to customize the
decorator for that particular element -- for instance, you may wish for
the Label decorator on one element to prepend, but append on another, or
add a specific id to a particular HtmlTag decorator. In your example
above, if you decided you wanted to customize the HtmlTag of one
element, you'd end up customizing it for all of them, as it would be the
same decorator instance across all elements.
So, in the end, while statics seem like they'd be a quick and easy
solution, they also end up making for less flexibility.
> I'll submit a unit test and a use case for the FormLabel improvement
> shortly.
I saw the comment come through in my inbox earlier; I'll look at it
after we do the final 1.5.0 release. Thanks for the contribution!
> On 03 Mar 2008, at 5:44 AM, Matthew Weier O'Phinney wrote:
>
>> -- Nathan <[EMAIL PROTECTED]> wrote
>> (on Tuesday, 11 March 2008, 04:00 PM -0700):
>>> First and foremost, hard coded default decorators seems suboptimal.
>>> Extending all the elements to define your own defaults isn't much of an
>>> option, since you'd have to subclass all the element classes. I realize
>>> the
>>> defaults can be bypassed by passing your own decorators to the element's
>>> constructor, but this isn't always ideal either.
>>
>> I want to point out a few things here. First, it's hardly "hard coding"
>> -- it's providing sane defaults. There *is* a difference -- hard coding
>> means you wouldn't be able to change them, and, quite clearly from your
>> own explanation, you can.
>>
>> Let's look at the flexibility availalble here (forgive me if I'm
>> repeating you a little...):
>>
>> * You can subclass and override the loadDefaultDecorators() method
>>
>> * You can pass in decorators when creating elements/forms/etc, which
>> will prevent the default decorators from loading
>>
>> * You can even prevent the default decorators from loading at all (by
>> passing a 'disableLoadDefaultDecorators' option at instantiation)
>>
>> * You can set all element or display group decorators at once
>> (setElementDecororators(), setDisplayGroupDecorators())
>>
>> I'm failing to see how this is not enough flexibility, but, whatever.
>>
>>> I propose that default decorators be loaded from a static array via
>>> addDecorators(). Then the static array could be overridden by calling
>>> Zend_Form_Element::setDefaultDecorators($array). Besides being more
>>> flexible, this also has the potential to improve performance, since it
>>> would allow you to set your own decorator *instances* and save a ton of
>>> redundant pluginloader calls. Would this work or am I missing something?
>>
>> You're missing a couple fundamental things, actually. First off, not all
>> elements use the same set of decorators. The Button, Submit, Reset, and
>> Image elements each have a different set of decorators than the other
>> elements -- and users can define their own elements with their own
>> defaults. If we use a static member for defining this array, then we
>> actually *lose* flexibility, as all elements would, out of necessity,
>> have to use the same set (either that, or we then end up potentially
>> having more logic and duplicate entries in classes that don't use the
>> same defaults).
>>
>> Additionally, your argument of using decorator instances will not work:
>>
>> class Foo
>> {
>> public static $decorators = array(
>> new Zend_Form_Decorator_ViewScript()
>> );
>>
>> public function getDecorators()
>> {
>> return self::$decorators;
>> }
>> }
>>
>> $foo = new Foo;
>> var_export($foo->getDecorators());
>>
>> Try the above -- you'll get a parse error. It's not valid PHP. That
>> means that the only way that using decorator instances would work with a
>> static member.... is if you add them in *MANUALLY* prior to
>> instantiating your objects. This sounds to me like about the same amount
>> of work as doing this:
>>
>> $decorators = array(
>> new Zend_Form_Decorator_Foo(),
>> new Zend_Form_Decorator_Bar(),
>> new Zend_Form_Decorator_Baz(),
>> );
>> $form->setElementDecorators($decorators);
>>
>> Basically, statics tend to reduce flexibility in the long run as any
>> class with static members ends up becoming a de facto singleton. They're
>> a pain to test, and even worse to extend.
>>
>> Honestly, I think there's plenty of flexibility already. The thing about
>> forms is, you create them once, and use them many times. Start extending
>> Zend_Form, and put your logic in your extending class -- then you have a
>> single place to debug it, it's not in your controller or model logic,
>> so you don't have to look at the details when debugging program flow.
>>
>>> My other thought was that the FormLabel View Helper should be able to
>>> accept a Zend_Form_Element, thus saving you from manually calling
>>> ZFE::getName() and ZFE::getLabel().
>>
>> If you use the Label decorator, you don't have to do this anyways...
>>
>>> I've created an issue on the tracker for this improvement (ZF-2865), and
>>> included a working patch, but I'm a little unsure of the preferred
>>> protocol
>>> here. Should I assign this to myself? Write a unit test? I couldn't find
>>> anything on the dev wiki which explains what's involved nor how you can
>>> help to the process along. Any insights would be appreciated, as this is
>>> the first open source project I've taken an interest in contributing to.
>>
>> Submit a patch that includes a unit test, and I'll review and (a)
>> determine if it's a feature we want to include, and (b) apply the patch
>> if we do.
>>
>> --
>> Matthew Weier O'Phinney
>> PHP Developer | [EMAIL PROTECTED]
>> Zend - The PHP Company | http://www.zend.com/
>
--
Matthew Weier O'Phinney
PHP Developer | [EMAIL PROTECTED]
Zend - The PHP Company | http://www.zend.com/
