Beefy Boxes and Bandwidth Generously Provided by pair Networks
XP is just a number
 
PerlMonks  

Safer monkey-patching

by tobyink (Abbot)
on Jan 18, 2012 at 22:51 UTC ( #948643=perlmeditation: print w/ replies, xml ) Need Help??

What is monkey-patching? It's the practise of adding methods to somebody else's class. Perl makes this very easy:

package Data::Dumper::Extensions; use Data::Dumper; sub Data::Dumper::print_r { print Dumper(@_); }

... but monkey patching is generally looked down upon. There are many good reasons - chiefly, what if a new version of Data::Dumper introduces its own print_r method? Worse still, what if the maintainers of Data::Dumper are discouraged from adding print_r because of worries of causing incompatibilities with your module?

The example above is somewhat contrived. There's not really any reason to define the print_r function in Data::Dumper - it could just as easily be in a separate package. Monkey patching becomes more useful when it's used to define new methods for object-oriented modules.

What is the alternative to monkey patching? In many cases, an easy alternative is to create a subclass of the class you are targeting and add your new methods there. Ain't OO great? Yes, except when it isn't.

The subclassing technique becomes less useful when you've got lots of objects which can instantiate each other. Say for example, you've got a class Example::ErrorList which is a list of Example::Error objects. Maybe you've subclassed Example::Error as My::AwesomeError... but Example::Error doesn't know this, and its most_recent_error method stubbornly returns plain old Example::Error objects.

This kind of situation makes monkey patching more attractive.

But how can we go about safer monkey patching? I think I've hit upon a good technique. Let's use the example of My::AwesomeError, which is a subclass of Example::Error, but adds an extra method asplode. (Example::Error already has a method called as_string.)

package My::AwesomeError; BEGIN { require Example::Error; push @Example::Error::ISA, __PACKAGE__; } sub as_string { die "as_string should be implemented by subclass"; } sub asplode { my $self = @_; die "BANG! ".$self->as_string; }

What we do here is set My::AwesomeError as a superclass or Example::Error. This ensures that $error->asplode will work on Example::Error objects, but also ensures that if Example::Error ever implements its own asplode method, that one will "win".

What do people think? Advantages? Drawbacks? Improvements?

Comment on Safer monkey-patching
Select or Download Code
Re: Safer monkey-patching
by Anonymous Monk on Jan 19, 2012 at 03:57 UTC

    For some reason I think this worse than just monkey patching, but I really can't pin down why -- conclusion, whatever floats your boat, its all the same to me

    update: After reviewing some of my recent monkey patching efforts ( htmltreexpather.pl, Re: HTML::TableExtract Memory Usage ), I figured out why I don't like messing with @ISA, it evokes feelings of action at a distance, after all it is one step removed, where as monkey patching is as straight forward as you can get. I also vaguely recall method lookup caching could survive @ISA updates.

    So , I think I'll stick to monkey patching :)

      Method lookup caching doesn't happen until a method has actually been called. Because the @ISA manipulation happens in a BEGIN block, this should occur before any method calls, thus before any method lookups have been cached.

      Also, according to perlobj any changes to @ISA should invalidate the cache. There may be bugs lurking somewhere in perl's method lookup caching mechanism, but none that seem to be triggered by the technique I've discussed.

      After all, the situation where package $X manipulates package $Y's <c>@ISA at BEGIN-time is really common. (Consider the case where $X eq "base" or $X eq "parent". Sure, this is happening at package $Y's request, but from Perl's perspective the same thing is happening.)

      With Moose it happens at run-time, and this doesn't seem to cause any problems.

        ... BEGIN ... perlobj ... parent ... perspective ..

        By the time you load Data::Dumper or whatever, resolution method could have happened

        But yeah, I did say I vaguely recall, esp because I didn't feel like digging up history or pinning down the details :)

        As for parent... perspective is exactly the point. If you're bothering to write a module to share, you might as well do it right, which is not dickering around with other peoples class data :) delegate (AUTOLOAD or ...)

        At least if you monkey patch, warnings will issue "Subroutine %s redefined"

        With Moose it happens at run-time, and this doesn't seem to cause any problems.

        Sure it does :) Moose Cookbook - Meta Recipe 5 Won't Run

      I also vaguely recall method lookup caching could survive @ISA updates.
      It can! If your perl is old enough. I think this bug got fixed in 5.004 or 5.005.
Re: Safer monkey-patching
by moritz (Cardinal) on Jan 19, 2012 at 05:34 UTC

    The big problem with monkey patching is that no two modules can safely monkey-patch the same third module. Recently there was a rather drastic example in the Ruby community when two large libraries or frameworks added incompatible json methods to Class (or was it Object? dunno...), and that meant you couldn't use these two libraries in the same process.

    Your approach doesn't solve this problem. And it can't, because it's not reasonably solvable.

    I might add that source filters suffer from the same problem as monkey patching: you can't combine two source filters fiddling with the same piece of syntax.

      Indeed, this problem is not really solvable. However, my solution does offer a partial way forward.

      Let's imagine that My::AwesomeError and Your::GroovyError both superclass Example::Error, and both add an asplode method.

      We can then do:

      my $err = Example::Error->new('Heat death of universe detected!'); $err->My::AwesomeError::asplode(); $err->Your::GroovyError::asplode(); $err->asplode(); # Russian roulette

      With true monkey patching, where My::AwesomeError and Your::GroovyError each defined the asplode method directly in the Example::Error namespace, we wouldn't get the option to do the above.

        We can then do:
        my $err = Example::Error->new('Heat death of universe detected!'); $err->My::AwesomeError::asplode(); $err->Your::GroovyError::asplode(); $err->asplode(); # Russian roulette

        Of course you can, but it's not really a benefit. As you wrote in the OP, monkey patching is often done to work around limitations in existing code that you don't want to patch. But writing $obj->Class::method() requires the author of that code to be aware of the issue, and when he is, he can simply write Class::method($obj) without requiring any monkey patching.

        I do find your idea cute, but it really doesn't solve the "hard" problem, and the small problems it does solve seem to be either artifically constructed, or non-issues.

Re: Safer monkey-patching
by dwalin (Monk) on Jan 19, 2012 at 06:30 UTC

    In my experience, monkey patching usually is not worth its trouble. The problem you describe above I would solve by subclassing + duck typing: in Example::ErrorList, provide a method that returns list item class name, and instantiate objects of that name. This way you can subclass both Example::ErrorList and Example::Error in a "clean" way.

    Like this:

    package Example::ErrorList; sub list_item_class { 'Example::Error' } sub new { my ($class) = @_; my $item_class = $class->list_item_class; my $self = bless {}, $class; $self->{items} = [ map { $item_class->new($_) } @seed_list_or_some +thing ]; return $self; } package Example::Error; sub as_string { die "as_string should be implemented by subclass"; } sub asplode { my ($self) = @_; die "BANG! ".$self->as_string; } package My::AwesomeErrorList; use base 'Example::ErrorList'; sub list_item_class { 'My::AwesomeError' } package My::AwesomeError; use base 'Example::Error'; sub as_string { # Actually implement it }

    You also have a typo: my $self = @_; won't work the way you expect.

    Regards,
    Alex.

      in Example::ErrorList, provide a method that returns list item class name, and instantiate objects of that name.

      You can also make that an attribute (plus accessor, if you want), so that subclassing isn't even necessary. If you use blessed hash references as objects, and do not create the hash key in the default case, there's not even a memory penality to be paid unless you deviate from the default.

      An example for a read and write accessor could be

      sub error_class { my $self = shift; my $default = 'Example::Error'; if (@_) { my $new = shift; if ($new eq $default) { delete $self->{error_class} } else { $self->{error_class} = $new' } } $self->{error_class} // $default }
Re: Safer monkey-patching
by choroba (Abbot) on Jan 19, 2012 at 09:51 UTC
    Isn't the Factory_design_pattern also a solution to this? (It requires a complete rewrite of the original class, though :-)

      If we were able to rewrite the original class, then we wouldn't be in this situation.

        Well, yeah. But if we write our classes using Factories, we do not get anyone into the situation.
Re: Safer monkey-patching
by JavaFan (Canon) on Jan 19, 2012 at 10:08 UTC
    n many cases, an easy alternative is to create a subclass of the class you are targeting and add your new methods there.
    Yet, it doesn't solve the same problem you mention as a drawback to monkey patching.

    Suppose you do subclass Data::Dumper (say, to My::Data::Dumper), and define a method My::Data::Dumper::print_r, and you use instances of My::Data::Dumper instead of instances of Data::Dumper. Now, what if a new version of Data::Dumper defines a method print_r, and calls that method in an OO way? Then in instances of My::Data::Dumper, My::Data::Dumper::print_r is called instead of the expected Data::Dumper::print_r. While not getting a warning or error at compile time (which you would get when monkey-patching), it doesn't seem to be likely that this is going to work out at runtime.

    What we do here is set My::AwesomeError as a superclass or Example::Error. This ensures that $error->asplode will work on Example::Error objects, but also ensures that if Example::Error ever implements its own asplode method, that one will "win".
    And that's all honkey dory? Sorry, I don't see what's so good about this. Sure, you will not get a compile time error, but where you first would die with a bang if you call $error->asplode, that's unlikely to happen once Example::Error defines its own asplode.

    I don't think there's a way to defend against future name clashes, assuming one party isn't aware of your pick of names. Of the three mentioned techniques (monkey patch, sub class, super class), the monkey patch will detect the name clash at compile time. The two OO variants will just silently do the wrong thing at run time.

      Redefining a function only produces a warning if "use warnings" is in effect. So classic monkey patching will not necessarily produce compile-time warnings.

      If a conflict warning is desired, that's fairly easy to add in with my method...

      BEGIN { require Example::Error; foreach my $method (qw/asplode/) { warn "Example::Error->$method already defined." if Example::Error->can($method); } push @Example::Error::ISA, __PACKAGE__; }
        Redefining a function only produces a warning if "use warnings" is in effect. So classic monkey patching will not necessarily produce compile-time warnings.
        Are you serious? You consider that a reasonable argument? "You shouldn't monkey patch because you may not get a warning on a name clash if you don't enable warnings"?

        I'm pretty sure that anyone who knows how to monkey patch knows about warnings.

        If a conflict warning is desired, that's fairly easy to add in with my method...
        Goody. Additional scaffolding, and you still aren't any further than what can be achieved with monkey-patching.

        Assuming Example::Error doesn't use AUTOLOAD to implement asplode (heh, if you want to consider monkey patchers that don't enable warnings, I will consider AUTOLOAD), it still doesn't solve anything. Once Example::Error implements asplode, you get a warning, and where your code expects to call your asplode, it calls Example::Error::asplode. Ergo, you haven't solved anything. It isn't safer than monkey-patching. With the extra scaffolding, it isn't unsafer either.

Re: Safer monkey-patching
by sundialsvc4 (Abbot) on Jan 19, 2012 at 16:39 UTC

    I quickly become nervous and neurotic when I become uncertain about “what will this code do?” and especially if I fear that it might do something different six months from now due to reasons that are beyond my control.

    My general approach to dealing with the problem is to build a set of application-specific “helper” objects, which spell out exactly what functionality my application does actually require, and which by some means “reflect” the requests that are made to them to whatever bit of code is (presently...) tasked with carrying it out.

    “Subclassing” and other fancy-pants ;-) methodologies (although of course I do routinely use them) make me nervous because they construct a system of mutual dependency that is just waiting on “the one exception to the rule” that will bring the Bill Ding™ (yes, the toys have a website...) come a’tumbling down.   “We just acquired who?”   “Marketing wants to do what?!?!”   :-O   Elegant software constructions can save a lot of time when they are written, but they are also mutually dependent upon one another, and those dependencies have a nasty way of becoming nasty.   I do not offer a generalized solution to the issue because I don’t think that one exists.   But “monkey patching” is specifically frightening to me because, even if it “solves” the problem now, I have no confidence that it will continue to solve the same problem forever.   I avoid the practice, and those similar to it, for these reasons.   You can only rely on Monster.Com to bail you out so many times before the word starts to get around.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://948643]
Approved by ww
Front-paged by ww
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others rifling through the Monastery: (5)
As of 2014-09-22 04:44 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (178 votes), past polls