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