Don’t overuse instance_eval and instance_exec


Not sure how known this antipattern is. I’ve seen some libraries that are very good at not doing it, but I’m also seeing lots of Ruby code that does use it without reason.

What I’m talking about here is the common usage of using instance_eval on a block to make it possible to use other methods inside of that specific block. If you want to do stuff based on method_missing inside of a block, this is the way people generally use.

So what’s the problem with it? Well, the problem is that blocks are generally closures. And you expect them to actually be full closures. And it’s not obvious from the point where you write the block that that block might not be a full closure. That’s what happens when you use instance_eval: you reset the self of that block into something else – this means that the block is still a closure over all local variables outside the block, but NOT for method calls. I don’t even know if constant lookup is changed or not.

Using instance_eval changes the rules for the language in a way that is not obvious when reading a block. You need to think an extra step to figure out exactly why a method call that you can lexically see around the block can actually not be called from inside of the block.

There are ways around instance_eval usage in a library – you can always assign self to a local variable outside of the block, and then call methods on that local variable. How ugly does that sound?

In almost all cases, if a block needs to handle method calls on a specific object, it should send that object in to the block as an argument. Take a look at routes in Rails – they could have been defined with instance_eval, but they’re not. There is no reason to use instance_eval for this case. Rails send in a route instead. File.open doesn’t use instance_eval with the block. It could, but instead it sends in the file. This is because there is no need to use instance_eval, and sending in the object as an argument gives the definition more power.

This doesn’t mean instance_eval should never be used. That’s not true, it’s a hugely useful feature, but it should definitely be used with good taste. If you’re unsure when to use it, don’t!



Short term adoption and Ruby DSLs


I feel like I’ve been picking a bit on DataMapper in my recent posts, so I first want to say that I like most of what I see in DataMapper, and is generally just nitpicking. I’m also what I’ve noticed there to give examples instead of making up something. And actually, one of the reasons is that it was some time since I read much Ruby code, and DataMapper happens to be one of the things I’m looking a lot at right now.

Which brings me to something Sam Smoot said in the discussion about adding operator methods to Symbol. (In the blog post Simplified finders in DataMapper). What he said was this:

That might satisfy some, but it ranks pretty low on the “beauty” gauge. And that really does matter for adoption…

Now, the reason I’m bringing it up here is not to pick on Sam or DataMapper (Really!). Rather, it’s because an attitude that I’ve noticed all over the Ruby world lately. In most cases it’s not explicitly said like this, but I like Sam’s quote because it verbalizes exactly what it’s about. You really want your library to look nice. Beauty is really important in a Ruby framework. It really is important, and lots of focus is put on it. And one of the reasons for this is to drive adoption.

Since it’s been verbalized like this, I’ve realized that this is one of the things that really disturbs me with the Ruby communities fascination with making everything, absolutely everything into a DSL.

Now, I really do like thinking about DSLs and working with them. Being a language implementor brings that out in me. But a DSL really has it’s place. Why shouldn’t you make everything into a DSL? Why shouldn’t everything look nice? First of all, the initial development might just not be worth it. A DSL-like approach generally involves more effort, so maybe a regular API works fine? A DSL introduces cost, both in development and in maintenance. Everything you do needs to be balanced. My coworker Marcus Ahnve gave a very good example of this today. He wanted to use ctags with an RSpec file, but since ctags doesn’t understand “describe” and “it”, there was no way for it to extract any useful information from it. Now, this is a trade of the framework designer has to make, and in the case of RSpec I think that the DSL is totally justified, but in other cases it’s not.

So take the issue at hand. Adding a few methods to Symbol, so that finding information in a database will look a bit nicer. In real terms this involves polluting the Symbol namespace with methods that have a very narrow scope. The usage (that looks more or less like this: Exhibition.all(:run_time.lt => 3)) uses something that reads easily from an English/SQL perspective, but not necessarily from a Ruby perspective. The main question I have when seeing this is what the eventual cost down the line will be for having it. Is this approach expandable? Is there any possibility of clashes with other libraries? Will someone else easily maintain it?

Focusing on beauty to generate adoption seems like the wrong choice for me. You add things that might not be so good long term, to get a larger initial code base. I would prefer to make the right choices for the right reasons first, and then let adoption be driven by that.

In summary, be responsible when designing APIs. I know those are very boring words. But it’s a reality, and your users will thank you for it.

PS: Sam just wrote a new comment, saying that the Symbol operators will be optional in the next DataMapper release. Good choice.