Dispatch from the front lines: What I learned today
I spent the last day of 2008 continuing my work to simplify and streamline the render path in Rails. That work involves moving all path generation to a single place (instead of requiring that various methods throughout the system generate paths and collect the appropriate templates), and creating a single point of entry into ActionView instead of the numerous points of entry that exist now.
While I was doing this work, I noticed something that is worth discussing in more detail. In Rails 2.x, ActionController takes calls to #render, converts them some, and then calls ActionView::Base#render. It calls into ActionView with an arbitrary hash that is more or less the same hash that was passed into ActionController, with a few changes. However, that same ActionView::Base#render method is the public-facing API for calls to render inside Rails views. As a result, any work that modifies ActionView::Base "internals" has an effect on both the public-facing API of ActionView itself as well as the quasi-private API used for inbound communication from ActionController.
"What I learned today" was that when writing code, there can be seemingly very strong (DRY-related) reasons to try to reuse code in a public interface for private purposes. Resist this temptation and create solid, simple, inter-class APIs. Trying to reuse public-facing APIs for private purposes can make future refactoring very difficult, and isn't really worth the (in the moment) gains.
On the bright side, once I deconstructed the problem, it didn't turn out to be very difficult to get in a refactoring groove. Effectively, the plan is to (1) create a simpler inter-class communication method; (2) slowly (very very slowly) start moving existing calls into the public method into the new method.
In case you're still interested, some specifics:
- Define some areas of responsibility: ActionController should be responsible for collecting all request-related information and passing it on to ActionView. ActionView should be responsible for aggregating the collected information and figuring out what needs to be rendered.
- Define an API:
- ActionController will collect (1) the template name to be generated (foo); (2) a list of acceptable formats (:html, :xml) based on the current request; (3) a prefix (the current controller name, "layouts", or something else that makes sense); (4) whether or not the template to be rendered is a partial. Note that this format works for render :file, render :template, render :action, and even render :partial.
- ActionView will receive the inbound information through a single method (provisionally called render_for_parts, but this name will likely change) and ask its PathSet (just a list of template paths--this part is completely transparent to the user) to return a template for the parts. The PathSet will handle converting the group of components into a path, and return back a Template object representing the template at the path location.
- ActionView will then call render on the template object, and the template object will return back an appropriate string (some details in this area are currently being worked out by Josh, so stay tuned for news from him).
- Note that the ActionController=>ActionView conduit is now reduced to a single method that takes a known set of 4 arguments. This conduit works for any call into ActionController::Base#render, and shifts responsibilities to where they belong (previously, ActionView was partly responsible for determine what the template format should be; this is now moved appropriately to ActionController, and is included in the arguments passed through the conduit).
- There is still some work required to make layouts work correctly, but I'm very pleased that the approach of creating a new API first and then pointing old methods at them one at a time turned out to be a very viable way to approach the problem. What this also means is that there are currently multiple copies of some logic in the codebase, but as I am able to completely remove dependencies on the old code paths, this will no longer be the case.
