RoR based MVC Web Frameworks – Friend or Foe?

Ruby on Rails has inspired a whole generation of MVC Web Frameworks. The RoR inspired frameworks follow the same basic design; Controllers whose public methods are web endpoints. So…Do RoR based MVC Web Frameworks encourage good design?

MonoRail and MS MVC are a part of the RoR generation.  

They must. Right? I show, in the code below, that RoR inspired frameworks can actually encourage bad design. I show, in the end, what I think is a better way.

The code presented here is written for MonoRail, but the same problems exist in MS MVC and RoR.  

Let’s start with a simple ProductsController. The index method of this controller is the implementation of http://whocares.com/products/<code&gt;. The code on the end of the URL is the code for a specific product (i.e. nj1287).

public class ProductsController : SmartDispatcherController{   public void Index(string code)   {      Product product = Product.FindByCode(code);      if(product == null)      {         PropertyBag["UnknownSearchTerm"] = code;         RenderView("common/unknown_quick_search");      }      PropertyBag["Product"] = product;      RenderView("display_product");   }}

CategoriesController is our next controller. The index method of this controller is the implementation of http://whocares.com/categories/<name&gt;. The name on the end of the URL is the name for a specific category (i.e. montiors).

public class CategoriesController : SmartDispatcherController{   public void Index(string name)   {      Category cat = Category.FindByName(name);      if(cat == null)      {         PropertyBag["UnknownSearchTerm"] = name;         RenderView("common/unknown_quick_search");      }      PropertyBag["Category"] = cat;      RenderView("display_category");   }}

The index method in both classes are basically the same. We can remove the duplication. Let’s extract an abstract class.

public abstract class SearchableController : SmartDispatcherController{   public void Index(string name)   {      ISearchable item = FindByName(name);      if(item == null)      {         PropertyBag["UnknownSearchTerm"] = name;         RenderView("common/unknown_quick_search");      }      PropertyBag["Item"] = item;      RenderView(IndexView);   }   protected abstract ISearchable FindByName(string name);   protected abstract string IndexView { get; }}public class ProductsController : SearchableController{   protected ISearchable FindByName(string name)   {      return Product.FindByCode(name);   }   protected string IndexView { get { return "display_product"; } }}public class CategoriesController : SearchableController{   protected ISearchable FindByName(string name)   {      return Category.FindByName(name);   }   protected string IndexView { get { return "display_category"; } }}

What do you think about our new DRY code?I don’t like it. Why?

  1. ProductsController and CategoriesController no longer “speak” to me.
  2. Inheritance. I really don’t like the template method pattern because of #1.

Can we fix this?We can use a DynamicAction in MonoRail. Let’s see that code.

Does RoR or MS MVC have a concept of a “DynamicAction”?  

public class IndexAction : IDynamicAction{   private ISearchableRepository repos;   private string indexView;   public IndexAction(      ISearchableRepository repos,      string indexView)   {      this.repos = repos;      this.indexView = indexView;   }   public void Execute(Controller controller)   {      string name = controller.Params["name"];      ISearchable item = repos.FindByName(name);      if(item == null)      {         controller.PropertyBag["UnknownSearchTerm"] = name;         controller.RenderView("common/unknown_quick_search");      }      controller.PropertyBag["Item"] = item;      controller.RenderView(indexView);   }}public class ProductsController : SmartDispatcherController{   public ProductsController()   {      DynamicActions["Index"] = new IndexAction(             new ProductsRepository(), "display_product");   }} public class CategoriesController : SmartDispatcherController{   public CategoriesController()   {      DynamicActions["Index"] = new IndexAction(             new CategoryRepository(), "display_category");   }}

I like this code better. We are no longer using inheritance to reuse code and the code “speaks” better to me. This code has two things that bug me.

  1. The controllers are really not doing much.
  2. DynamicActions are second class citizens in MonoRail.

How do we fix these issues? I don’t know how in the RoR inspired frameworks. I think we need another abstraction. Chris Ortman and I discussed the below code at HDC 07 (so he deserves at least part of the credit). I am not sure he agreed, but maybe this post will change his mind.

The code below is not apart of any framework. It is only an idea.  

public class IndexAction : SmartDispatcherAction{   private ISearchableRepository repos;   private string indexView;   public IndexAction(      ISearchableRepository repos,      string indexView)   {      this.repos = repos;      this.indexView = indexView;   }   public void Execute(string name)   {      ISearchable item = repos.FindByName(name);      if(item == null)      {         PropertyBag["UnknownSearchTerm"] = name;         RenderView("common/unknown_quick_search");      }      PropertyBag["Item"] = item;      RenderView(indexView);   }}

IndexAction is now a first class citizen of the framework. Actions have all the benefits that controllers have in MonoRail (parameter binding, …).How do we hook this action to a URL?

new Route("/products/<name>",    new IndexAction(new ProductRepository(), "display_product"));new Route("/categories/<name>",    new IndexAction(new CategoryRepository(), "display_category"));

Routing engines allow us to remove the “controller” completely. We don’t need a “controller” (at least not in the RoR framework form). What were our controllers doing? They were really just organizing our actions. Routing engines allow us to organize our actions without the “controller”.What do you think? I would really like some feedback on this topic. Does anyone else feel this pain with these frameworks?

Advertisements

7 Responses

  1. Hi Adam,

    This approach also has a name. It’s called the Command pattern 🙂

    What’s nice about controllers is that actions are logically grouped and so it’s easy to do things like call one from another and attach filters at the controller level. Additionally, it is trivial to factor out common code into private controller methods.

    I wonder whether your controller extract superclass refactoring example is really how most people would DRY this code up?

    Cheers,

    Andrew.

  2. @Andrew

    You are right. It is the command pattern.

    I am not sure I understand how you would DRY this up using private methods. I want to DRY across controllers not in the same controller. It seems like private methods only work in the same controller.

    AE

  3. Adam,

    I wasn’t suggesting that you could DRY this particular code up using private methods, merely that being able to share private methods across logically grouped actions was handy.

    I was suggesting that using inheritance here could perhaps be overkill. In RoR, for example, I suspect there is very little use of controller subclassing. In this case, a simple mixin could do the job. Of course, that’s Ruby for you 🙂

    Oh and as for the Command pattern idea it’s a good one and quite popular already, particularly in Java land. Check out WebWork for one example.

    Cheers,

    Andrew.

  4. Andrew,

    I don’t think a Mixin works here either. A Mixin is simply a way to copy a method(s) to the controller. I am pretty sure RoR has the same issues I am pointing out here.

    You can see more explanation about these issues in the castle dev discussion group.

    http://groups.google.com/group/castle-project-devel/browse_thread/thread/976170df54fde8f5

    AE

  5. Hi Adam,

    I am interested to know what you would think about using MVP with Commands instead. I don’t particularly dislike MVC, but I’m wondering if Microsoft really needs to kick everything out and start from scratch to give us the features we really want.

    I actually think that the controllers will lead to worse code, as I can see people mixing up LINQ statements, presentation code, and all kinds of loose stuff in the PropertyBag.

    Plus, the controller has code for all pages pertaining to a model, not just for one page. I don’t find that necessary nor helpful.

    I am thinking of building a simplified version of the Patterns and Practices MVP pattern, and making it available on CodePlex. What do you think of that? Would it be useful?

    I’d appreciate your comments, as I’m currently involved in blog-discussions with Scott Hanselman and Rob Conery about this very subject.

    Best regards,
    Richard

  6. Richard,

    I think MVC and MVP can both be used with single action controllers (commands/actions). In fact, I think that in order to maximize maintainability and DRYablity single action controllers should be used in most cases.

    Do I think a simplified MVP pattern would be useful? Maybe. I think the best thing that can be done is to get out the message that multi-action controllers are not the best way to build large maintainable web applications.

    Are we on the same page? Or have I totally missed your point.

    AE

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: