[OT] Wrapping methods

[Andy Wardley]

Nicholas Clark wrote:
> Of course this can also be solved in various other ways,
[...]
> but that doesn't feel as elegant an interface.

Perhaps not, but IMHO having two distinct methods is the Right Way To Do It.
Anything else runs the risk of being Too Clever By Far[1].

I would have an external run() method to act as an interface and an internal
_run() method to provide the implementation.

     sub run {
         # setup
         $self->_run(@_);
         # cleanup
     }

That's the simple, vanilla OO approach.  You can tart it up with a bit of
Moose magic if you like, but that should be the dressing on top, not the
cake itself.

That said, I think a better approach in this situation would be to implement
a generic call_method_with_timeout() method/function that you can inherit
from a base class, import from an external module, or mixin using some other
fancy technique.

     sub call_method_with_timeout {
         my ($self, $method, @args) = @_;

	# setup				# your timeout code
         $self->$method(@args);		# goes around this
         # cleanup			# bit here
     }

You can then implement run() as a simple call to the above method.

     use Your::Utils 'call_method_with_timeout';

     sub run {
	# easily skimmable, self documenting code, yummy.
         shift->call_method_with_timeout( _run => @_ );
     }

If a slick API is your thing then another approach is to use a monad object
to represent the timeout wrapper.  Something like this perhaps:

     package Method::Timeout;
     our $AUTOLOAD;

     sub _bind_ {
         my ($class, $object) = @_;
         bless \$object, ref $class || $class;
     }

     sub AUTOLOAD {
         my $self = shift;
         my ($name) = ($AUTOLOAD =~ /([^:]+)$/ );
         return if $name eq 'DESTROY';

         # setup                          # your timeout code
         my $result = $$self->$name(@_);  # goes around this
         # cleanup                        # bit here

         return $result;
     }

Then in your object class (or imported from another module)

     use Method::Timeout;

     sub timeout {
         Method::Timeout->_bind_(@_);
     }

The timeout() method returns a Method::Timeout object wrapped around your
$self object.  This wrapper object then delegates all method calls to the
original object, but with the timeout alarm set.

The end result is that you can then call any method with a timeout wrapper
like so:

    $object->timeout->run(@args);

You can also pass additional parameters to the timeout method if you like,
e.g.

    $object->timeout(10)->run(@args);

You can also reuse the timeout object if you need to:

    my $timeout = $object->timeout(10);
    $timeout->foo();
    $timeout->bar();
    $timeout->baz();

And it also works with class methods (as does call_method_with_timeout()):

    my $object = My::Class->timeout->new( connect => $a_slow_server );

I find this approach rather satisfying.  It clearly separates "the method
that you're calling" from "the way you want the method to be called".
Separation of concerns and all that.

Badger::Base implements the try() method this way.  It simply puts an eval { }
wrapper around the method you're calling, effectively downgrading a thrown
exception to an undefined return value [2].

    if ($object->try->some_method(@args)) {
        print "success\n!";
    }
    elsif ($@) {
        print "failed: ", $object->error, "\n";
    }
    else {
        print "declined: method returned false value, no error thrown\n";
    }

It's not as elegant as proper try/catch blocks, but I think it's nicer
than the Old Skool approach of wrapping eval { } blocks around the method
call.

    if (eval { $object->method(@args) }) {   # Not as nice, IMHO
        ...
    }

But I digress...

Whichever way you do it, I think it's important to separate the underlying
action being performed (the _run() method or whatever you call it) from the
additional timeout behaviour that is orthogonal to it.

Cheers
A


[1] i.e. clever enough to confuse others

[2] Returning undef to indicate failure is a Bad Thing.  Have your methods
     throw exceptions by default and let the caller make the decision to
     ignore/handle them (using try(), eval { } or whatever) when appropriate.
     (for the grandmas who don't already know how to suck eggs :-)