views:

98

answers:

1

Core CS question here: of the Design Patterns listed in Gamma, etc, which (if any) cover monkeypatching? Additionally, for what class of problems is monkeypatching appropriate vs. subclassing? Patching bugs in core library classes is one, are there others? I hear lots of sturm und drang about monkeypatching on stackoverflow, most of you seem to have strong misgivings about it, but as a programmer I really like the ability to encapsulate generic bits of functionality and include them in my object models in rails.

Take thoughtbot-paperclip, for example, why would I want ever want to subclass that vs the monkeypatch approach that exists today?

Thanks, -Eric

+1  A: 

I don't think monkeypatching is a Design Pattern - core class extension a language feature that they seem to ignore.

Regarding the rest, have a look at Jeff Atwood's views on this article on his blog.

In his (and my) oppinion the biggest problem with monkeypatching is that it can make debugging very difficult, if it modifies existing methods - we humans can't keep track of all the "little snippets here and there" as well as machines do. Subclasses stablish clearer separations.

So my personal rules for monkeypatching are:

  • If you can do it without monkeypatching and it will work ok, don't use monkeypatching.
  • You can add new methods to classes, but you can't modify existing ones.
  • Do it in a very visible and obvious way - i.e. a file called string_extensions.rb in your /lib, not on a hidden /myvendor/submodule/se.rb .
  • Should be local; classes that don't use a library should be unaffected.

Now, to your example: Paperclip.

  • To my knowledge, it adds methods to your ActiveRecord classes, but doesn't modify existing ones
  • You have to add a has_attachment directive to the classes that use paperclip, otherwise they are not affected.

So the changes are localized and obvious (I actually think that it manages to improve debugging: is easier for us humans to read has_attachment instead of class MyModel < Paperclip::ActiveRecordWithAttachment).

Subclassing is also a bad idea on this case because you would not be able to use paperclip in addition to another plugin that used subclasses - rails is single-inherited.

And in Paperclip's case, it is clearly a has_a relationship with its attachment, not a is_a one. One could argue that it would not be a proper use of subclassing.

Finally, I'd like to point out that Paperclip requires subclassing in some occasions (you have to use subclassing in order to create a paperclip processor).

egarcia