AaronCrane.co.uk

Monkey patching, subclassing, and accidental overriding

The problem with monkey patching

Suppose you’re working on some software which uses a third-party class. Maybe it’s open-source, maybe it’s commercial software, doesn’t matter. But one day you think to yourself, “hmm, wouldn’t it be nice if this class had some more methods that would make it fit better with what I’m trying to do?”.

Now, there are two basic ways of accomplishing that. One is to subclass the third-party class, and add some new methods in the subclass. The other, at least for languages with dynamic type systems, is to just stuff the methods you want into the existing class — what’s known as monkey patching. Now, if you’ve been down that road before, you’re probably cringing right now, but it’s apparently a popular approach, so let’s just take a look at a couple of relatively well-known examples where people have done that.

Here’s one from early 2008. Prototype is a JavaScript library which has often taken the approach of adding methods to standard JavaScript classes, where it feels they’re useful. For example, they add an each method to arrays, to simplify iterating over an array’s elements. They also had an early implementation of the getElementsByClassName method, which finds all the HTML elements in the current page that have some particular CSS class.

Now, Prototype is actually pretty careful about how it injects methods. In particular, it never adds a method that already exists. That means that, if a browser starts natively implementing a method that Prototype wants, the browser’s native version is used in preference to the Prototype version.

So people building web apps with Prototype happily started using getElementsByClassName where necessary:

var dull = document.getElementsByClassName('dull');
dull.each(Element.hide);

And then Firefox 3 and Safari 3.1 started providing native implementations of getElementsByClassName. But up pops the law of unintended consequences. The native implementations return a standard NodeList representing the matched elements, while the Prototype implementation returns a plain Array of elements. And the Prototype Array.each method isn’t available on NodeList, so bang! code like this broke when end-users upgraded their browsers.

Here’s another one, for a different language. Ruby on Rails added a new method to strings, called chars, which returned a “multibyte-safe” version of the same string:

# Rails <= 2.1
mb_safe = "some string".chars
# ==> "some string"

But then in August 2008, Ruby 1.8.7 added a completely different method of the same name, intended to let you iterate over the characters of a string:

# Ruby 1.8.7
biggest_char = "some string".chars.max
# ==> "t"

So when people using the Rails version of chars upgraded from Ruby 1.8.6 to Ruby 1.8.7, their apps stopped working.

At the time, the consensus response to problems like this was “you should always use subclassing instead of monkey patching”. Sadly, as I’m going to show, that response has two problems. First, it can be way more effort than monkey patching, especially for things like the standard string class in a language; I’ll come back to that issue later on. Second: it doesn’t actually solve the problem!

The problem with subclassing

What do I mean by that? Let’s look at the Perl ORM module DBIx::Class (which I’m a huge fan of, by the way). Suppose you’ve got a database of books, and you want to count how many distinct authors there are. In plain old DBIx::Class, that might look like this:

say $schema->resultset('Book')->search_rs(undef, {
    columns  => ['author_id'],
    distinct => 1,
})->count;

That’s quite a lot of code for something relatively simple, and it has the annoying feature of hiding the “count” bit at the end of a very long method call. Eventually you hit on the idea of writing a method to simplify that:

say $schema->resultset('Book')->count_distinct('author_id');

Being a well-behaved sort of programmer, you write a subclass which does nothing but add that method:

package My::DBIC::ResultSet;
use base qw<DBIx::Class::ResultSet>;

sub count_distinct {
    my ($self, $column) = @_;
    return $self->search_rs(undef, {
        columns  => [$column],
        distinct => 1,
    })->count;
}

Then you jump through the hoops necessary to make sure your subclass is used in all the appropriate places, and you write some tests, and they all pass, and you decide your work here is done.

Wonderful.

Then, some time later, the sysadmins keeping your app running decide it’s time to upgrade DBIx::Class. Mostly, you can expect that to go smoothly, because the DBIx::Class developers do a fine job, and they have pretty good test coverage. But you can’t guarantee it. Suppose the DBIx::Class developers have also been annoyed by the inconvenience of this “count distinct” thing, and they’ve added their own method with a slightly different API — perhaps their version takes any number of columns, not just one:

# Hypothetically…

package DBIx::Class::ResultSet;

sub count_distinct {
    my ($self, @columns) = @_;
    return $self->search_rs(undef, {
        columns  => \@columns,
        distinct => 1,
    })->count;
}

Note that the tests for your version of count_distinct will still pass, because everything your method handles is also handled by the new upstream method. And the tests for the new version of DBIx::Class itself also pass, because when they’re run, your code isn’t even loaded.

But suppose that this new version of DBIx::Class also includes another method which invokes count_distinct with multiple column names:

# in package DBIx::Class::ResultSet

sub blah {
    my ($self, ) = @_;
     $self->count_distinct(@multiple_columns) 
}

If that method gets invoked on a plain DBIx::Class::ResultSet, all will be well. But if it’s invoked on an instance of your subclass, you’ll silently get the wrong behaviour. That is, no matter how much work you went through to create a subclass rather than just monkeying your method into the existing class, you still made things break!

Fixing the subclass problem

This can be expected to happen in pretty much any programming language, including languages that are much less dynamic than Perl. All it takes is for you to take a third-party class, subclass it with an extra method, and then later on, a new version of the third-party class adds a method with the same name as yours, and uses it somewhere in its own implementation. In effect, the distinction between the two versions of the third-party class allows for a peculiar sort of temporal dynamism in method resolution, even in statically-typed languages. But the great thing about Perl compared to less powerful languages is that we can find a way to fix it.

We want to prevent this sort of accidental overriding, but how? It seems absurd to directly legislate against overriding methods in derived classes; after all, sometimes that’s just what you want. Let’s look at the various cases. The simplest thing is defining a method in a class with no superclasses:

package Base;

# A plain old method
sub m1 { ... }

That’s obviously fine, of course. Similarly, adding a new method in a derived class is fine:

package Derived1;
use base qw<Base>;

# Not defined in Base — a new method
sub m2 { ... }

It’s also fine to write a derived-class method which knows about the base-class version — that can’t be accidental:

package Derived2;
use base qw<Base>;
use mro 'c3';  # for next::method

# Implemented in terms of the inherited Base::m1 method
sub m1 {
    my ($self, @args) = @_;
    $self->do_something_else;
    $self->next::method(@args);
}

The only tricky case is something like this:

package Derived3;
use base qw<Base>;

# Same name as a method in Base.
# Deliberate or accidental?
sub m1 { ... }

Clearly, there isn’t enough information there to work out whether this m1 method override is deliberate or accidental. And for that matter, even with the m1 in Derived2, which uses next::method to extend the behaviour of the base-class version, it’s hard to observe from the outside that that’s what you’re trying to accomplish.

Method modifiers

It seems we need more precision in describing our classes. Well, it turns out that some object systems give you just that. Moose builds on the Class::MOP meta-object protocol to give you “method modifiers”. For example, Moose lets you write the Derived2 example this way:

package Derived2;
use Moose;
extends qw<Base>;

# Modify the inherited Base::m1 method
before m1 => sub {
    my ($self) = @_;
    $self->do_something_else;
};

So, rather than directly overriding m1 with a new version that coincidentally happens to do a new thing before going back to the original implementation, we specifically say that this is a simple modification of the inherited method. There’s a similar after modifier which does just what you’d expect, and an around modifier which is a bit like combining the two. And there’s also an override modifier which simply replaces the inherited method:

package Derived3;
use Moose;
extends qw<Base>;

# Deliberately override the inherited Base::m1 method
override m1 => sub { ... };

These method modifiers are exactly what we need for distinguishing deliberate and accidental method overriding. You can decide that you’ll reserve the plain method-definition syntax for defining methods that you don’t expect to exist in your superclasses, and that you’ll always use the Moose method modifiers when you want to deliberately override or extend a method. Then it’s possible to programmatically detect accidental overriding by interrogating the MOP.

(You may also find that the discipline of relying on method modifiers actually makes your code clearer: before is a much more precise statement about what you’re doing than the equivalent implemented with next::method.)

Checking for implicit overrides

So, how do we write code that determines whether a method has been implicitly overridden? Let’s start with a function named check_class which takes a class name as argument, and throws an exception if that class overrides any method in its inheritance graph other than with an explicit method modifier:

use Class::MOP;

sub check_class {
    my ($checkee) = @_;
    my $meta = Class::MOP::Class->initialize($checkee);
    my @overrides = grep { implicitly_overridden($meta, $_) }
        $meta->get_method_list;
    die "Implicit overrides in class $checkee: @overrides\n"
        if @overrides;
}

That finds all the methods implemented directly in $checkee, and checks whether each one is an implicit override. The heavy lifting is done by implicitly_overridden. Here’s a sample implementation; it might not be completely bullet-proof, but it’s a good proof of concept:

use List::MoreUtils qw<none>;

sub implicitly_overridden {
    my ($meta, $method_name) = @_;
    my $method = $meta->get_method($method_name) or return 0;
    return $meta->find_next_method_by_name($method_name)
        && none { $method->isa($_) } qw<
            Class::MOP::Method::Meta
            Class::MOP::Method::Wrapped
            Class::MOP::Method::Generated
            Moose::Meta::Method::Overridden
        >;
}

That takes a metaobject representing a class, and the name of a method which might be defined in that class. It then finds the metaobject representing the method of that name (or bails out if no such method exists). Then it considers the method to have been implicitly overridden if (a) there’s an inherited method of the same name, and (b) the locally-defined method wasn’t defined using a method modifier.

With that code, it’s quite easy to put together a pragma-like module that lets individual classes opt in to this checking. (In practice, this sort of checking has to be opt-in, so that you can add it piecemeal to an existing class hierarchy.) The idea is that a class which wants this error checking can say

use explicit::overrides;

and guarantee to get a compile-time error if it accidentally overrides any base-class methods.

Implementing that is surprisingly straightforward:

package explicit::overrides;

my @CLASSES_TO_CHECK;
CHECK { check_class($_) for @CLASSES_TO_CHECK }

sub import {
    my (undef, @args) = @_;
    croak "explicit::overrides takes no import arguments"
        if @args;
    push @CLASSES_TO_CHECK, scalar caller;
}

On import, we merely store the name of the importing class for later. Then at CHECK time — immediately before the main program starts executing — we do the necessary checks on all the classes that have asked for it.

Overriding in other languages

As an aside: if you’re familiar with other languages, you may be wondering what relationship, if any, this explicit::overrides pragma has to the Java @Override annotation, or the C# override declaration.

The answer, sadly, is: almost none.

Let’s look at the sort of example that motivates Java @Override. Suppose you have a base class Base with some method meth taking an int:

class Base {
    public void meth(int x) {
        System.out.println("In Base#meth " + x);
    }
}

Now you subclass Base, and override that method:

class Derived extends Base {
    public void meth(int x) {
        System.out.println("In Derived#meth " + x);
    }
}

So far, so good. However, at some point, the base class author decides that it would be better for meth to take a double argument, rather than an int, and updates the class accordingly:

class Base {
    public void meth(double x) {
        System.out.println("In Base#meth " + x);
    }
}

At this point, the Derived class still compiles, but its void meth(int) method doesn’t actually override anything, because the base-class version has a different signature. This is a form of what’s known as the fragile base class problem.

There are much worse versions of this problem. For example, if you’re adding a mouse event listener to some UI component, you may at first try this buggy code:

someUIComponent.addMouseListener(new MouseAdapter(){
    public void mouseEntered() {  }  // bug!
});

The bug is that the mouseEntered method you’re trying to override takes a MouseEvent argument — so this anonymous subclass adds a fresh method which never gets invoked.

I’m sure you can see how that sort of bug would be hard to track down.

Newer versions of Java therefore add the @Override annotation: if a method is declared @Override, then it’s a compile-time error if there isn’t a method in the class’s inheritance graph with the same signature. Most Java pundits recommend adding @Override to every method definition that you expect to override an inherited method; if you follow that discipline, then bugs of this sort lead to compile-time errors:

someUIComponent.addMouseListener(new MouseAdapter(){
    @Override
    public void mouseEntered() {  }  // compile-time error
});

However, Java doesn’t take the next step: to assume that a class which overrides an inherited method without @Override is also an error. It’s easy to see why not: @Override is optional, and that has to be the case because it wasn’t available before Java 1.5. So while @Override seems useful, it doesn’t directly help with the problem I’m discussing here.

C# does a little better than this: you can declare a virtual method as either override (like Java @Override) or new (so that invoking the appropriate method name through a base-class reference calls the inherited version, not the derived-class replacement). And the compiler even gives you a diagnostic if you override a method with neither override nor new. Unfortunately, though, the diagnostic is a warning, not an error, so this still doesn’t achieve our goal of getting compile-time errors for accidentally-overridden methods.

Fixing the monkey-patch problem

Regardless of the flaws in other languages, we can feel pretty pleased with ourselves at this point: a little discipline when writing a subclass, and a couple of dozen lines of code in the explicit::overrides module, and we can guarantee that upgrading to an incompatible version of the base class will cause a compile-time error.

But remember the other problem with using subclassing instead of monkey patching: it can be really awkward to make sure your subclass is used everywhere appropriate. That’s especially true in situations like the Ruby String#chars method: you want your new method to be available on ordinary string literals, and on strings returned from arbitrary code. Perl has overload::constant to help with that, but it doesn’t help you when you’re looking at extending one specific class buried in the middle of a large system of cooperating classes — like with DBIx::Class::ResultSet.

So can we do something similar for monkey patching? Well, it turns out that’s actually easier: because we’re just dropping a method into a class, we don’t need a MOP to work out whether the class already has a method of that name. Let’s look at how we might want override-safe monkey patching to work:

use monkeypatched 'DBIx::Class::ResultSet' => (
    count_distinct => sub {
        my ($self, $column) = @_;
        return $self->search_rs(undef, {
            columns  => [$column],
            distinct => 1,
        })->count;
    },
);

We have a new pragma-like module called monkeypatched which takes a class name and a hash from method names to code refs. It should load the class, and then inject the code refs into that class under the appropriate names. But if the class already has access to some method of that name, it should throw an exception. Implementing that turns out to be pretty easy:

package monkeypatched;

use Carp qw<croak>;

sub import {
    my (undef, $target, %methods) = @_;
    eval "CORE::require $target; 1" or die $@;
    while (my ($name, $code) = each %methods) {
        croak "$target already has a method '$name'"
            if $target->can($name);
        install_sub($target, $name, $code);
    }
}

The install_sub function does exactly what you’d think — it names the code ref appropriately, and then inserts it into the appropriate package:

use Sub::Name qw<subname>;

sub install_sub {
    my ($package, $name, $code) = @_;
    my $full_name = "$package\::$name";
    my $renamed_code = subname($full_name, $code);
    no strict qw<refs>;
    *$full_name = $renamed_code;
}

This code is already available on CPAN, under the name ex::monkeypatched (where the ex:: prefix just indicates that the API is still considered experimental), and I’m actually using it in production at my company. I really like it. I can trivially inject methods into existing classes, without having to find suitable ways of writing and using subclasses. But I can sleep soundly in the knowledge that this monkey patching is safe even if one of my dependencies is upgraded incompatibly. The error happens at compile time, so just trying to load my code will fail — which means I can be certain my tests will fail. (And, of course, I’d never upgrade anything on live servers without testing under the new version on a dev box first.)

Conclusions

The conclusions from all this are possibly surprising. First, it’s a little odd to find that subclassing is no safer than monkey patching. And second, even though they can both be dangerous if done without care, a sufficiently dynamic and flexible language lets you make both of them entirely safe.

So much for the alleged danger of monkey patching!