Re: [Perl-widget-developer] thoughts on design

[Stephen A. <ste...@of...>]

Cees,

1. I am encouraged that someone is looking at the code.
   Good for you Cees.
2. Your suggestions are good, but your patches conflict with things
   that I am working on.  I understand that it was worth it to you
   just to play with the code.

See below for comments.

Stephen

At 10:30 PM 6/2/2001 -0500, ce...@si... wrote:
...
>First off, any options we pass to a constructor should probably be in a
>hash, so the user doesn't have to worry about order of variables:
>
>$anniv_dt = $wc->widget(-name => "anniv_dt");
>instead of
>$anniv_dt = $wc->widget("anniv_dt");
>

Good idea, but I don't like being forced to use named parameters for
everything when a single parameter (i.e. $name) is what is almost always
expected.

Per Gunther, I considered CGI::Util::rearrange(), and opted for the 
rule: "when first char of first arg is dash (-), use named args".
We may go to rearrange(), but it seems like a lot of overhead that we
don't need. (CGI.pm had a huge installed base using an old syntax to
support.)

>I noticed this morning that someone had patched the Controller Classes to
>accept a hashref in the constructor, however, I think this should be a
>hash instead of a hashref (for simplicity of the interface).
>
>$wc = Widget->controller({cgi => $query});
>Should become
>$wc = Widget->controller(cgi => $query);

Yeah.  I agree.

>In the first release, the Widgets were conpletely driven by hte Config
>object.  I have added some support that allows the widgets to be
>configured at initialization time as well as from the Config object.  
>This allows someone to create a Widget as such:
>
>    $widget = $wc->widget(
>        -name       => 'category',
>        -class      => 'Widget::HTML::Select',
>        -values     => ['Foo', 'Bar'],
>    );

I knew people would want to do this, and I have been working on adding
it myself.  However, I consider this usage the "low road".  It encourages
people to do exactly what the library is supposed to get them away from ...
caring about the details of the widget.  It also inhibits the configurer
from snapping a new implementation of a widget in to upgrade the page.

However, this is "simpler" and I expect that we will have "high road"
users (using the library more ideally) and "low road" users (employing
short cuts which somewhat short-change the real value of the library).

Actually this philosophy is in keeping with Perl. ;-)
There's more than one way to do it.

>This change to allow widgets to be defined dynamically has another
>implication.  In the first release, none of the widgets did much at
>initialization time.  They did not read the config file until a rendering
>function was called (ie ->html()).  If we are passing config info through
>the constructor, then all this initialization will have to be done in the
>constructor.

>This will greatly simplify the html() function, but make the init()
>function more complex.
>
>I have also gone ahead and redesigned the DateDropDowns.pm to use the
>child/parent structure.  When DateDropDowns is initialized, it creates 3
>new widgets and stores them in the object.  These widgets are not listed
>in the Config file, since DateDropDowns should know how to configure them
>automatically:
>
>    $self->{year} = $self->{controller}->widget(
>        -name       => $self->{name}.'.year',
>        -class      => 'Widget::HTML::Select',
>        -controller => $self->{controller},
>        -parent     => $self,
>        -values     => [$start_year..$end_year],
>    );
>    $self->{month} = ...
>    $self->{day}   = ...
>
>and I added three helper function that will allow the user to access these
>widgets directly.  This allows someone to do the following:
>
>$anniv_dt   = $wc->widget(-name => "anniv_dt");
>$anniv_dt->value();  # Something like yyyy-mm-dd
>$anniv_dt->year->value();   # just returns or sets the year part
>
>I also noticed a small bug in Widget/Base.pm with the value() function
>(The @_ was no longer being sent to the state->value function.

I found this too.

>sub value {
>    my $self = shift;
>    my $value = $self->{controller}->state()->value($self->{name}, @_);
>                                                                 ^^^^
>}
>
>The @_ was missing, and should be included if someone wants to set the
>value of the widget.
>
>I have included a patch file that will implement the above changes.  I
>hope they don't conflict with anyone elses changes.  I understand that I
>should have discussed what I was working on before implementing these
>changes, but this was a good way for me to learn the code, and see where
>it could use improvements.

Sorry, but I can't use the patch files because of conflicts.
I did look at them.

>If this patch file is too big, or conflicts with other changes, I don't
>mind breaking it up, or redoing it.  I have a sourceforge account
>(ceeshek), but I don't know if you want to give everyone CVS access or
>not...  If not, I will continue to send patches, otherwise, I will discuss
>my changes on the list, and patch the tree myself.

I have added you as a developer (CVS write access).
However, please do not assume your changes will not conflict without
discussing it first on the mailing list.
And do not commit anything without first getting an OK on the mailing list.

>Also, please note that I am working from Sydney Australia, so my messages
>to the list may come at strange hours :)
>
>I think this project has had a very positive start, and I am looking
>forward to seeing it develop into an easy, yet powerful tool.
>
>Cees Hek
>

Thanks.

Stephen