Yehuda Katz is a member of the Ember.js, Ruby on Rails and jQuery Core Teams; he spends his daytime hours at the startup he founded, Tilde Inc.. Yehuda is co-author of best-selling jQuery in Action and Rails 3 in Action. He spends most of his time hacking on open source—his main projects, like Thor, Handlebars and Janus—or traveling the world doing evangelism work. He can be found on Twitter as @wycats and on Github.

Better Module Organization

As I said in my last post, Carl and I have been working on a more modular version of ActionController. As we fleshed out the feature set, we had a few needs that aren’t directly addressed by Ruby’s built-in feature-set.

  • Modules occasionally depended on other modules (there’s no point in having Layouts without Renderer), but including Renderer into Layout meant that we couldn’t have setup on Renderer that got applied to the controller class itself. In this case, Renderer adds a “_view_paths” inheritable accessor to new Controller classes that is used to store a list of paths containing templates. If we included Renderer into Layouts, and then Layouts into ActionController::Base, that setup would happen on Layouts, which is wrong.
  • We used the def self.included(klass) klass.class_eval { ... } end idiom a whole lot. In fact, that’s the only thing we used the included hook for, except…
  • Extending ClassMethods onto the class.

When I was at Locos X Rails, I spent some time with Evan, and he argued that using the included hook should be done only after trying other abstractions. After speaking for a few minutes, Evan suggested abstracting away the above ideas in a higher-level abstraction that wrapped include. We could then more directly control the inclusion process, and even add our own hooks where needed.

I ended up with:

module AbstractController
  module Callbacks
    setup do
      include ActiveSupport::NewCallbacks
      define_callbacks :process_action
    end
    ...
  end
end

replacing:

module AbstractController
  module Callbacks
    def self.included(klass)
      klass.class_eval do
        include ActiveSupport::NewCallbacks
        define_callbacks :process_action
        extend ClassMethods
      end
    end
    ...
  end
end

For dependencies, I replaced:

module AbstractController
  module Helpers
 
    def self.included(klass)
      klass.class_eval do
        extend ClassMethods
        unless self < ::AbstractController::Renderer
          raise "You need to include AbstractController::Renderer before including " \
                "AbstractController::Helpers"
        end
        extlib_inheritable_accessor :master_helper_module
        self.master_helper_module = Module.new
      end
    end
    ...
  end
end

with

module AbstractController
  module Helpers
    depends_on Renderer
 
    setup do
      extlib_inheritable_accessor :master_helper_module
      self.master_helper_module = Module.new
    end
    ...
  end
end

And finally, the Base controller itself could now be replaced with:

module ActionController
  class Base2 < AbstractBase
    use AbstractController::Callbacks
    use AbstractController::Helpers
    use AbstractController::Logger
 
    use ActionController::HideActions
    use ActionController::UrlFor
    use ActionController::Renderer # just for clarity -- not required
    use ActionController::Layouts
  end
end

from:

module ActionController
  class Base2 < AbstractBase
    include AbstractController::Callbacks
    include AbstractController::Renderer
    include AbstractController::Helpers
    include AbstractController::Layouts
    include AbstractController::Logger
 
    include ActionController::HideActions
    include ActionController::UrlFor
    include ActionController::Layouts
    include ActionController::Renderer
  end
end

It’s not a tremendous change, but it definitely reduces the likelihood of accidental mistakes, and makes the actual usage a lot clearer. Of course, we will need to document this new mechanism, but it has already simplified the necessary mental model of the setup.

As always, thanks for reading!

8 Responses to “Better Module Organization”

I like the idea of abstracting repeated idioms like included/class_eval thing but what’s the point of redefining the “include” method to “use” if it does the same thing, Does is check that the included module wasn’t previously included ? Else what’s the point ?

Do thoses module helpers gets its own separate lib of does it fall into ActiveSupport ?

I like it. I’ve run into this problem a lot myself, and I might have to borrow some of this.

So what does the use method look like? Does it, unlike the original code, include the dependencies rather than erroring out? It seems so based on your last code snippets.

If so, are there potential problems when you don’t know exactly what modules are being included? It seems useful to know that AbstractController::Renderer is included, and with this code (if it does automatically include dependencies) I’d have to know what all of the modules’ dependencies are in order to determine what functionality I’ve got mixed in. Not a hard thing to do, obviously, but I wonder what your thoughts are.

Also, I’m a bit confused as to why ActionController::Renderer is not required in the first example but required in the second, but that may be due to a lack of context.

As an aside, your blog is great; this stuff is very interesting to me! I’d be even happier with more frequent posts *nudge nudge*.

@antonin it does quite a bit more than the normal include, which I described a bit. For one, it supports dependencies, which cannot be done with include because the included hook is run AFTER the module is included (which confounds the ability to override methods in a dependency and call super).

It also simplifies the included hook/class_eval pattern. The point is to make it a lot easier to mentally map what’s going on, without a bunch of boilerplate that you have to grok every time.

Just out of curiosity, when you guys going to merge 3-0 branch and all your forks to rails/master?

Do you think it’s possible to release 3.0 in May?..

Ace!

I’ve been thinking for a while now, that the “acts_as_*” pattern would be a whole lot nicer if it was: SomeClass.acts_as(List) instead of SomeClass.acts_as_list

That way any object with the right sub-modules can be a behavior and as an end-user I have some notion of how the code is organized. There may be conventions within the acts_as_* world, but when the strategy is let library authors decide how to reach into core classes it’s one heck of a crapshoot.

hello Yehuda,

I’ve been advocating a better use of modules as weel last september, quit glad to see those ideas getting into 3.0

My suggestion at the time, for what it’s worth, was to put the class base methods in a module called “module core” since they are de-core-ated. It made sense with the design pattern it is using.

Check my post here:
http://ruby.simapse.com/2008/10/rails-refactoring-exercise.html

I used active record for this rails refactoring exercise.
Trying this out with the AR i remember seeing the all tests pass… to my great surprise.

I’ll look more closely to the hole setup and use dsl.

Funny, I was just updating my own ‘use’ library tonight.

http://shards.rubyforge.org/wiki/wiki.pl?Use

1.3.0 is out.

Leave a Reply

Archives

Categories

Meta