Cees Hek wrote:
On 6/18/05, Rhesa Rozendaal <[EMAIL PROTECTED]> wrote:
- add_callback() accepts both a coderef and a string. Class::Trigger
only accepts coderefs. Why did you decide to accept sub names as well?

I did this because runmodes accept a string or a coderef, so it would
be consistent with the rest of CGI::Application.  Also, it makes
subclassing easier.

Yes, Thilo just pointed that out as well, and I think that's important. Since I already made add_callback() a wrapper around add_trigger(), I managed to support that after a fashion.

Here's the gist of it:

 use Class::Trigger;
 sub add_callback {
        my ($pkg, $hook, $method) = @_;
        $pkg->add_trigger($hook, sub{ shift()->$method(@_) } );
 }
 sub call_hook {
        my ($pkg, $hook, @args) = @_;
        $pkg->call_trigger($hook, @args);
 }
 sub new_hook {}

 # register default triggers
 __PACKAGE__->add_callback('init',     'cgiapp_init');
 __PACKAGE__->add_callback('prerun',   'cgiapp_prerun');
 __PACKAGE__->add_callback('postrun',  'cgiapp_postrun');
 __PACKAGE__->add_callback('teardown', 'teardown');

I also commented out the original code for the callbacks, of course (there's a complete diff at the end of this post).

This works, to a degree. All tests pass, except the more complex cases in t/11callbacks.t. In my runs the order is different, and I see some oddities: certain callbacks are executed more than once; not all expected events actually occur. Maybe I took too much of a shortcut in my add_callback() wrapper, or maybe the cgiapp contains more smarts. I'm not really in the mood anymore to figure it out right now.


- CT doesn't have the analog of new_hook(); it just creates the hook
point when add_trigger is called for the first time. That looks sensible
to me at first sight. What was the reason for the explicit new_hook()?

To me it acts more like 'use strict'.  It catches typos and silly
mistakes that can be very hard to track down.

Ah, so it warns if an undefined hook gets called? I suppose that's good, yes.

Dependancies don't worry me that much, as long as we are using 60-80%
of the functionality of the module.  If we are using a tiny piece of a
huge module, then I might speak up against it.

Class::Trigger itself is roughly 95 lines of code. It in turn depends on Class::Data::Inheritable, of which I don't know the size.

So far this is just a weekend excursion for me. If there's any feedback,
I might start to take it more seriously :-)

Thanks for looking into it.  I don't think all of this is set in stone
yet (but the cement is drying fast).  We have a basic system of
callbacks in place that I think will work for most people.  It will
take some time to cater to every possibility though.

Thanks for the reply.

I liked the ease with which I was able to refactor it. Class::Trigger is definitely worth considering in general. I'm not all that certain anymore it would win us much for cgiapp though. The current code does the exact same thing, and we're (well, Mark is ;) in full control.

I had fun poking around in all this. I'll attach this patch for whoever is interested in taking it further.

Have a great weekend!

Rhesa



--- Application.pm.orig 2005-06-19 04:20:21.000000000 +0200
+++ Application.pm      2005-06-19 04:53:34.000000000 +0200
@@ -4,8 +4,11 @@
 use Carp;
 use strict;
 use Class::ISA;
+use Class::Trigger;

-$CGI::Application::VERSION = '4.01';
+$CGI::Application::VERSION = '4.01_01';
+
+=for old code

 my %INSTALLED_CALLBACKS = (
 #      hook name          package                 sub
@@ -16,6 +19,23 @@
        load_tmpl => { },
 );

+=cut
+
+sub add_callback {
+       my ($pkg, $hook, $method) = @_;
+       $pkg->add_trigger($hook, sub{ shift()->$method(@_) } );
+}
+sub call_hook {
+       my ($pkg, $hook, @args) = @_;
+       $pkg->call_trigger($hook, @args);
+}
+sub new_hook {}
+
+__PACKAGE__->add_callback('init',     'cgiapp_init');
+__PACKAGE__->add_callback('prerun',   'cgiapp_prerun');
+__PACKAGE__->add_callback('postrun',  'cgiapp_postrun');
+__PACKAGE__->add_callback('teardown', 'teardown');
+
 ###################################
 ####  INSTANCE SCRIPT METHODS  ####
 ###################################
@@ -1953,6 +1973,8 @@

 =cut

+=for old code
+
 sub add_callback {
        my ($self_or_class, $hook, $callback) = @_;

@@ -1974,6 +1996,8 @@

 }

+=cut
+
 =item new_hook(HOOK)

     $self->new_hook('pretemplate');
@@ -1989,12 +2013,16 @@

 =cut

+=for old code
+
 sub new_hook {
        my ($class, $hook) = @_;
        $INSTALLED_CALLBACKS{$hook} ||= {};
        return 1;
 }

+=cut
+
 =item call_hook(HOOK)

     $self->call_hook('pretemplate', @args);
@@ -2021,6 +2049,8 @@

 =cut

+=for old code
+
 sub call_hook {
        my $self      = shift;
        my $app_class = ref $self || $self;
@@ -2060,6 +2090,8 @@
        }
 }

+=cut
+
 =pod

 B<Callback Ordering>


---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/[email protected]/
             http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to