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.
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
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:
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:
So, what are your favorite patterns for writing controllers?
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
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.
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
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?
13 comments:
Like your new site design by the way. Does look nice.
I use a larger font size than normal though - and blogger fixes the width in pixels rather than font size, so looks rather 'thin' at home.
IronPython URLs has the same problem though.
Anyway - not yet convinced by the "thin controller fat model" argument, at least not as it applies to non-Rails apps.
It *would* help with creating new controller layers for Resolver One (an alternative UI on a different platform) - but a lot of code would need rewriting whether it is in the model or the controller.
I think our recent experience at Resolver One shows that we need a reasonably lean *core* model structure, and another layer that mediates between the controllers and the core data representation.
I'm also not convinced by the "thin controller fat model" argument.
I think this is where Rails could be improved (or my lack of knowledge of the framework) because it doesn't separate Application Logic (AL) from Business Logic (BL).
In a traditional n-tier web architecture, the AL is triggered by URLs and instantiate the appropriate BL object, which, as its name implies, operates the real logic of your application. That way you can share the BL among all the representations of the result. (REST,SOAP, anything)
I implemented this in PHP, so I don't see how it couldn't be done in RoR or any other upper-class framework. What do you think ?
I really like this style. How would you implement a transaction? Have the transaction in one of the models and raise an exception? I always feel that crossing models like this is some how "dirty".
I sort of agree with the notion of skinny controllers and fat models -- at least in terms of tiers. However, Rails encourages a fairly narrow view of what constitutes a model. In a typical rails app, developers tend to think of models mainly as database backed data objects, and this often leads to shoving all the business logic into these data objects, rather than finding a more appropriate place for it by breaking things out into other classes (i.e. classes that are part of the model layer, but don't represent ActiveRecord models themselves). The default setup works great for CRUD based / RESTful apps, but once an app becomes more complex, the business logic should probably be extracted into proper, more granular domain objects, some of which may represent ActiveRecord models, while others may not.
It's usually a good idea to inherit custom exceptions from StandardError, not Exception.
A "rescue" that doesn't specify the exceptions it rescues will catch StandardError and all its subclasses.
Defining a communication protocol between models and controllers.... Great stuff!
Just taking a trip down memory lane, since I'm doing all my development work with Django and Python on the Google App Engine, but I am able to bring a few of my Rails lessons over to that world...
The RESTful way I do it is:
LIST shows all the items.
SHOW looks at one of those items.
EDIT modifies that item; it creates if needed, and also updates if validation is passed.
NEW just sends over to EDIT without an id.
DELETE does its work.
So that's all I need to implement REST in Django (Python) - 5 methods, not 7!
I realize code formatting is a personal thing, but if this was my project I'd have written 4b as:
credit_card = CreditCard.new(params[:credit_card])
flash[:message] = "success"
flash[:message] = "credit card validation error" unless credit_card.valid?
flash[:message] = "payment_failure unless order.capture_payment(credit_card)
redirect_to order
Yeah, I spend much time before I realized this rules. For 4-th rule I choose 4c as the most convenient way. Thanks for the post!
In 4a you ask if the state of the object is 'saved', in the 4b, you ask if the state of the credit card is 'valid'.
In both cases, the return codes are true or false. Therefore, 4a and 4b are very similar to my eyes.
I agree that version 4c is very clean and readable, but it troubles me in that it uses exceptions for things that are not (in my opinion, at least--perhaps some would disagree) really exceptions.
I understand exceptions to be failures, errors, cases that your code fails to account for, but needs to recover from. A credit card being declined is a very normal condition which you need to handle, not really an exception.
If we don't honor this distinction, your more readable code might be less readable 6 months from now, because glancing down the page you see exception handling and (unless you look at it closely and think about what it's doing) assume it's really there for handling exceptions.
what if I need to handle 2 naturally not exceptional returns?
Post a Comment