Is "monkey patching" really that bad?

21,447

Solution 1

Wikipedia has a short summary of the pitfalls of monkey-patching:

http://en.wikipedia.org/wiki/Monkey_patch#Pitfalls

There's a time and place for everything, also for monkey-patching. Experienced developers have many techniques up their sleeves and learn when to use them. It's seldom a technique per se that's "evil", just inconsiderate use of it.

Solution 2

Monkey-patching, like many tools in the programming toolbox, can be used both for good and for evil. The question is where, on balance, such tools tend to be most used. In my experience with Ruby the balance weighs heavily on the "evil" side.

So what's an "evil" use of monkey-patching? Well, monkey-patching in general leaves you wide open to major, potentially undiagnosable clashes. I have a class A. I have some kind of monkey-patching module MB that patches A to include method1, method2 and method3. I have another monkey-patching module MC that also patches A to include a method2, method3 and method4. Now I'm in a bind. I call instance_of_A.method2: whose method gets called? The answer to that can depend on a lot of factors:

  1. In which order did I bring in the patching modules?
  2. Are the patches applied right off or in some kind of conditional circumstance?
  3. AAAAAAARGH! THE SPIDERS ARE EATING MY EYEBALLS OUT FROM THE INSIDE!

OK, so #3 is perhaps a tad over-melodramatic....

Anyway, that's the problem with monkey-patching: horrible clashing problems. Given the highly-dynamic nature of the languages that typically support it you're already faced with a lot of potential "spooky action at a distance" problems; monkey-patching just adds to these.

Having monkey-patching available is nice if you're a responsible developer. Unfortunately, IME, what tends to happen is that someone sees monkey-patching and says, "Sweet! I'll just monkey-patch this in instead of checking to see if other mechanisms might not be more appropriate." This is a situation roughly analogous to Lisp code bases created by people who reach for macros before they think of just doing it as a function.

Solution 3

As long as the changes are isolated to your systems (e.g. not part of a software package you release for distribution) is there a good reason not to take advantage of this language feature?

As a lone developer on an isolated problem there are no issues with extending or altering native objects. Also on larger projects this is a team choice that should be made.

Personally I dislike having native objects in javascript altered but it's a common practice and it's a valid choice to make. If your going to write a library or code that is meant to be used by other's I would heavily avoid it.

It is however a valid design choice to allow the user to set a config flag which states please overwrite native objects with your convenience methods because there's so convenient.

To illustrate a JavaScript specific pitfall.

Array.protoype.map = function map() { ... };

var a = [2];
for (var k in a) {
    console.log(a[k]);
} 
// 2, function map() { ... }

This issue can be avoided by using ES5 which allows you to inject non-enumerable properties into an object.

This is mainly a high level design choice and everyone needs to be aware / agreeing on this.

Solution 4

It's perfectly reasonable to use "monkey patching" to correct a specific, known problem where the alternative would be to wait for a patch to fix it. That means temporarily taking on responsibility for fixing something until there's a "proper", formally released fix that you can deploy.

A considered opinion by Gilad Bracha on Monkey Patching: http://gbracha.blogspot.com/2008/03/monkey-patching.html

Solution 5

With regards to Javascript:

is there a good reason to avoid it otherwise, assuming that you are adding to the interface and not changing existing behavior?

Yes. Worst-case, even if you don't alter existing behavior, you could damage the future syntax of the language.

This is exactly what happened with Array.prototype.flatten and Array.prototype.contains. In short, the specification was written up for those methods, their proposals got to stage 3, and then browsers started shipping it. But, in both cases, it was found that there were ancient libraries which patched the built-in Array object with their own methods with the same name as the new methods, and had different behavior; as a result, websites broke, the browsers had to back out of their implementations of the new methods, and the specification had to be edited. (The methods were renamed.)

If you mutate a built-in object like Array on your own browser, on your own computer, that's fine. (This is a very useful technique for userscripts.) If you mutate a built-in object on your public-facing site, that's less fine - it may eventually result in problems like the above. If you happen to control a big site (like stackoverflow.com) and you mutate a built-in object, you can almost guarantee that browsers will refuse to implement new features/methods which break your site (because then users of that browser will not be able to use your site, and they will be more likely to migrate to a different browser). (see here for an explanation of these sorts of interactions between the specification writers and browser makers)

All that said, with regards to the specific example in your question:

For example, it might be nice to add a an Array.map implementation to web browsers which don't implement ECMAScript 5th edition

This is a very common and trustworthy technique, called a polyfill.

A polyfill is code that implements a feature on web browsers that do not support the feature. Most often, it refers to a JavaScript library that implements an HTML5 web standard, either an established standard (supported by some browsers) on older browsers, or a proposed standard (not supported by any browsers) on existing browsers

For example, if you wrote a polyfill for Array.prototype.map (or, to take a newer example, for Array.prototype.flatMap) which was perfectly in line with the official Stage 4 specification, and then ran code that defined Array.prototype.flatMap on browsers which didn't have it already:

if (!Array.prototype.flatMap) {
  Array.prototype.flatMap = function(...
    // ...
  }
}

If your implementation is correct, this is perfectly fine, and is very commonly done all over the web so that obsolete browsers can understand newer methods. polyfill.io is a common service for this sort of thing.

Share:
21,447
maerics
Author by

maerics

“Programs must be written for people to read, and only incidentally for machines to execute.” ― Harold Abelson, Structure and Interpretation of Computer Programs

Updated on May 17, 2020

Comments

  • maerics
    maerics about 4 years

    Some languages like Ruby and JavaScript have open classes which allow you to modify interfaces of even core classes like numbers, strings, arrays, etc. Obviously doing so could confuse others who are familiar with the API but is there a good reason to avoid it otherwise, assuming that you are adding to the interface and not changing existing behavior?

    For example, it might be nice to add a an Array.map implementation to web browsers which don't implement ECMAScript 5th edition (and if you don't need all of jQuery). Or your Ruby arrays might benefit from a "sum" convenience method which uses "inject". As long as the changes are isolated to your systems (e.g. not part of a software package you release for distribution) is there a good reason not to take advantage of this language feature?