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

[Gunther B. <gu...@ex...>]

At 10:30 PM 6/2/2001 -0500, ce...@si... wrote:

>I have a few comments on the design of the code base.  I have really only
>focussed on the Widget side of things and haven't really looked into the
>Controller/State/Config stuff too closely.  Also, I learn by playing with
>the code, so I have included a patch file of my suggested changes that I
>discuss below.
>
>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");
>
>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);
>
>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 like the familiar _rearrange() method that CGI.pm uses. I think it's 
simple and supports all the ways people tend to want to use methods.

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

 From a design pattern perspective, I have to say that this is better 
design. Constructions and Configuration methods (init) should by nature be 
more complex than the methods that perform operations.

This is entirely necessary so that the method signature for performing 
actions can remain the same throughout the code and only the part at the 
start of the code (which determines how the object is constructed) is changed.

Think of DBD drivers as an example. Each one accepts different connect() 
arguments but for the most part you can assume (other than SQL differences) 
that the methods remain the same.


>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 think this is reasonable.

>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.
>Also, this function should probably trigger a 'change' event if the value
>is being changed.
>
>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.
>
>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.
>

In principle I personally like the changes.

>Also, please note that I am working from Sydney Australia, so my messages
>to the list may come at strange hours :)

Not that strange of a timezone for me.

We started our open source company in Singapore last year. Exactly 12 hours 
ahead of eastern standard time in the USA. :)

Later,
    Gunther