MVC: How to write controllers

I've been working on many projects (web and desktop applications), and I must say that the way MVC is implemented varies a lot. That's probably a good thing, since every project is different. I believe, however, that there are some common patterns that could improve most of the MVC applications.

1. Keep it simple

Don't create additional tiers unless they are needed. Every class should clearly belong to either Controller, Model or a View.


2. Thin controller, fat model

This is probably the most useful lesson I learnt from the Rails world. I'm not going to elaborate on that here, as it was covered in many other places. Just move all your logic to models. It's that simple.


3. RESTful design

If you work on a web application, and still don't know what REST is, then go and read about it. It's easy and it will simplify your design a lot. Basically, it makes you think about your app as a set of resources. Every action in your controllers does one of the following with your resources: new, create, edit, update, show, destroy, index(list). At first, it can feel as a limitation, but it is the kind of limitation that actually helps you.

Many people argue that not everything can be RESTified. I agree that there are places where REST is not needed, however it's always a good exercise to find a RESTful solution.


4. Communication between controllers and models

As always, when you delegate some work further, you need to control the result somehow. The same happens when you move all the logic from controllers to models.

There are three ways (that I'm aware of) of implementing the communication between a controller and a model.

4a. Return codes

You use return codes when your model methods return not the data, but the result of an operation.

An example here is ActiveRecord::Base.save method which returns true when the save was successful, false otherwise.

order = Order.new(params[:order])
if order.save
  flash[:message] = "The order was created!"
  redirect_to order
else
  flash[:errors] = "Something was wrong"
  redirect_to new_order_path
end

In my opinion it's a good solution for simple situations. However, I don't like the "if" condition here and also it can go messy with more complex examples.

4b. Ask the object for its state

credit_card = CreditCard.new(params[:credit_card])
if credit_card.valid?
  if order.capture_payment(credit_card)
    flash[:message] = "success"
  else
    flash[:message] = "payment failure"
  end
else 
  flash[:message] = "credit card validation error"
end
redirect_to order

This is slightly better than a), because the return code knowledge is hidden in method calls, but we still see the problem of many "if" statements.

4c. Don't ask, tell. Use custom exceptions

That's my favorite, because it eliminates all the "if"s from my controller code. I know it's just a syntactical change, but I prefer to see something like this in my controller, rather than the example above:

begin
  order.pay(credit_card)
  flash[:message] = "success"
rescue CreditCardNotValid
  flash[:message] = "credit card validation error"
rescue PaymentFailed
  flash[:message] = "payment failure"
end

Here we use custom exceptions to define a "protocol" for the communication between a model and a controller. Obviously we need to define the exceptions somewhere. I like to add them at the top of the associated model class:

class CreditCardNotValid < Exception
class PaymentFailed < Exception

class Order < ActiveRecord::Base
  def pay(credit_card)
    ...
  end
end

So, what are your favorite patterns for writing controllers?

Git: working with branches

The way we work with git is that for every remote pair programming session we create a separate branch. We give it a name after the feature we're working on.

git checkout -b feature1

It automatically switches to this branch.

During our work we tend to checkin our changes quite often. We do it with:

git checkin -a -m "created some specs for class A"

After we finish our session, we do two things.
First, we merge our branch to master:

git checkout master
git merge feature1

Then, we delete the branch we were working on:

git branch -D feature1

That's it.

If you happen to delete the branch BEFORE you merge it, don't panic, there is a solution.
In order to undelete a branch just after you deleted it, do:

git checkout -b new_branch_name HEAD@{1}

Andrzej's Rails tips #11

Today I'm going to show you two tips, both related to Rails controllers.

Use the current_user object whenever you access its data

Instead of

@order = Order.find(params[:order_id])

do this:

@order = current_user.orders.find(params[:order_id])

Thanks to that, you don't have to check whether this order belongs to the user, you just need to handle ActiveRecord::RecordNotFound exception.

Move all the logic from your controller to the model

I know you already read this statement many times, but I will repeat it anyway.
In your actions you shouldn't manipulate your model objects, do it in the model class itself. Here's a simple example:

BAD CODE:

class OrdersController < ApplicationController
 def update
   @order = current_user.orders.find(params[:order_id])
   if params[:order][:amount] > 0
     @order.prepare_invoice
     @order.send_email
     @order.mark_as_paid
     @order.notify_producers
   end
 end
end

BETTER CODE:

class OrdersController < ApplicationController
 def update
   @order = current_user.orders.find(params[:order_id])
   @order.pay(params[:order][:amount])
 end
end

class Order < ActiveRecord::Base
 def pay(amount)
   if amount > 0     
     prepare_invoice
     send_email
     mark_as_paid
     notify_producers
   end
 end
end

Andrzej's Rails tips #10

form_for and namespace route

When you use a namespace route like the following:

  map.namespace :admin do |a|
    a.resources :users
  end

then if you want to use form_for @user, this is the correct way:

<% form_for([:admin, @user]) do |f| %>

2 minutes with David Chelimsky and RSpec stories (video)

David Chelimsky at Railsconf 2008 from Gregg Pollack on Vimeo.

Story Driven Development

If you are interested in testing and the TDD/BDD movements, then you may find Bryan Helmkamp presentation on "Story Driven Development" worth watching. Bryan explains the differences between unit testing and scenarios. He also shows Webrat, a tool he works on, which makes defining stories in Rails apps very easy.

I like the title that Bryan has chosen for the talk. The name "Story Driven Development" makes it clear that it's a lot about defining requirements, and not only about testing. Additionally, I think it's clear that it focuses on the acceptance level of tests without mentioning unit tests.

He also provides a quote from Robert Martin:

Unit tests: “Make sure you write the code right.”
Scenarios: “Make sure you write the right code.”

You can watch the slides here, or grab the PDF version (which has correct code examples).

| View | Upload your own

"TDD with Rails" - the slides from my talk at RuPy 2008

| View | Upload your own