Evil hook methods?

September 7th, 2008

I have come to realize that there are a few hook methods I really don’t like in Ruby. Or actually, it’s not the hook methods I have a problem with – it’s the way much code is written using them. The specific hooks that seems to cause the most trouble for me is included, extended, append_features and extend_features. Let me first reiterate – I don’t dislike the methods per se. The power they give the language is incredible and should not be underestimated. What I dislike is the way it makes things un-obvious when reading code that depends on them.

Let’s take a silly example:

module Ruby;end

module Slippers
  def self.included(other)
    other.send :include, Ruby
  end
end

class Judy
  include Slippers
end

p Judy.included_modules

Since all this code is in the same place, you can see what will happen when someone include Slippers. And really, in this case the side effect isn’t entirely dire. But imagine that this was part of a slightly larger code base. Like for example Rails. And the modules were spread over the code base. And the included hook did a few more things with your class. No way of knowing what – except reading the code – and the Ruby idiom is that include will add some methods and constants to your class and that is it. Anything else is going outside what the core message of that statement is.

One of the most common things you see with the included hook is something like this:

module Slippers
  module ClassMethods
  end

  def self.included(other)
    other.send :extend, ClassMethods
  end
end

class Judy
  include Slippers
end

This will add some class methods to the class that includes this module. DataMapper does this in the public API, for example. It’s very neat, you only have to include one thing and you get stuff on both your instances and your classes. Except that’s not what include does. Not really. So say you’re debugging someone’s code and happen upon an include statement. You generally don’t check what it’s doing until you’ve exhausted most other options.

So what’s wrong with having a public API like this?

module Slippers
  module ClassMethods;end
end

class Judy
  include Slippers
  extend Slippers::ClassMethods
end

where you explicitly include the Slipper module and then extend the class methods. This is more obvious code, it doesn’t hide anything behind your expectations, and it also might give me the possibility to choose. What if I want most of the DataMapper instance methods, but really don’t want to have finders on my class? Maybe I want to have a repository pattern? In that case I’ll have to explicitly remove all class methods introduced by that include, because there is no way of choosing if I want to have the class methods or not.

So that’s another benefit of dividing the extending out from the included hook. And finally, what about all those other things that people do in those hooks? Well, you don’t really need it. Make it part of the public API too! Instead of this:

module Slippers
  def self.included(other)
    do_funky_madness_on other
  end
end

class Judy
  include Slippers
end

make it explicit, like this:

module Slippers;end

class Judy
  include Slippers
  Slippers.do_funky_madness_on self
end

This is really just good design. It makes the functionality explicit, it makes it possible for the user to choose what he wants without doing monkey patching. And it makes the code easier to read. Yeah, I know, this will mean more lines of code. Booo hooo! I know that Ruby people are generally obsessed with making their libraries as easy to use as possible, but it feels like it sometimes goes totally absurd and people stop thinking about readability, maintainability and all those other things. And really, Ruby is such a good language that a few more lines of code like this still won’t make a huge impact on the total lines of code.

I’m not saying I haven’t done this, of course. But hopefully I’ll get better at it. And I’m not saying not to use these methods at all – I’m just saying that you should use them with caution and taste.

By Ola Bini | In: Uncategorized | tags: design, metaprogramming, readability, ruby. | #

16 Comments, Comment or Ping

Charles Oliver Nutter

I know the evils of hook methods all too well from recent work untangling Java integration. In order to make interfaces from Java feel like Ruby modules, there’s some pretty nasty included hooks, and they’re very fragile. It took me a bit of time to even figure out where the code was coming from.

September 7th, 2008
Caligula

Hear hear; powerful idioms require both restraint and clarity.

September 7th, 2008
Stephen Celis

Agreed. Just because a convention is established does not mean it’s always the clearest; i.e., “include” should probably not extend a class. I hope more people adopt this mindset.

September 7th, 2008
rick

Why not put the include line in the #do_funky_madness_on method? That way it’s still only one line.

September 7th, 2008
Hongli Lai

Agreed. Magic is good but only until a certain level.

September 7th, 2008
Dr Nic

If the issue is clarity of “what is happening here?” perhaps you could alias #include to be #include_and_extend which you would use instead to tell the reader that including this module is going to also extend the class?

For example:

class Judy
include_and_extend Slippers
end

Users of Slippers would be instructed/recommended in documentation to use the #include_and_extend method to enhance readability and understanding.

Might help.

[the 1st paragraph of this comment is also on Jay’s blog article reply]

September 8th, 2008
David Chelimsky

@nic – I think that would likely just complicate things. I’d be willing to lay a wager that within just a month of the Ruby community adopting this convention somebody will use include_and_extend to only include instance methods.

WDYT?

September 8th, 2008
Lou Scoras

This would probably be better if include passed any additional parameters as user parameters to the hook method–instead it takes additional modules. That way the caller could choose what kind of functionality gets pulled into the including module:

class Judy; include Slippers, :with => [:class_methods, :funky_madness] end

September 8th, 2008
Sam Smoot

I really don’t see an issue here with #include. It’s too common an idiom at this point.

And the class-method extension is basically becase some people (Jon Tirsen for one) wanted a non-inheritance based solution. It’s really not meant to separate instance and class methods. It’s a package deal.

I think the real complaint here is that DM mixes in the responsibility of Mapping, some persistance logic, and the UnitOfWork in the Resource. So it violates 1RR pretty significantly.

And that’s not ideal. I admit it. Right now I’m more focused on releasing 1.0. It’s an eventual goal to figure out how to break out some of those concerns a bit better. Ironically they were broken out in the 0.3.x line, but the 0.9.x line is much better more flexible code just the same (outside of associations anyways :p ).

So yeah. You want to use an external Mappings, UnitOfWork and make Repository the Mapper. So do I. The concern about ::included seems like a time-waster IMO.

September 9th, 2008
calu

“…It’s a package deal.”

Maybe that’s it, instead of Slippers call it SlippersPackage or something so that the user knows that what he’s included is much more that it seems.

class Judy
include SlippersPackage # or SlippersAndFriends
end

September 9th, 2008
Ola Bini

Charles:

Haha, yeah, those were BAD. Seriously bad. I feel your pain there.

September 9th, 2008
Ola Bini

Rick:

Sure, that would be much better, and I would prefer it.

September 9th, 2008
Ola Bini

Nic:

That would solve part of the problem, yes, if it would be possible to get people to do it. The problem is the semantics of the name. include_and_extend Slippers would in my mind mean the equivalent of doing

include Slippers
extend Slippers

while it would generally involve something like

include Slippers
extend Slippers::ClassMethods

September 9th, 2008
Ola Bini

David:

Yeah, that’s what I’m afraid of too.

September 9th, 2008
Ola Bini

Sam:

Well, let me be the first to tell you that I hate inheritance much more than I hate the include-with-extend idiom. But just because include-with-extend is getting more common doesn’t mean it’s right. And for me the problem is the long term benefits of a technique. From a readability perspective it’s just not right.

And I don’t agree about it being a waste of time to concern myself with included. Totally opposite actually – the more people use the idiom, the worse it will be to use libraries with it. Personally I prefer the approach Jay gives in his blog – defining a method Object#data_mapper_resource. Possibly append an exclamation mark to show that it’s modifying the receiver. The expectations on what a method will do is totally different from an include, which means it’s natural to assume it includes more behavior than just an include.

Of course, if you’re just including instance methods, include is the right way to do it.

To me this just feels like sound software engineering, and something the Ruby community doesn’t focus enough on.

September 9th, 2008

Hook method | Olpera - July 11, 2012

Reply to “Evil hook methods?”

« WordPress posting malfunctioning on Orange

Paradigms of Artificial Intelligence Programming (in Ruby) »

September 7th, 2008