MVC: Delegating interface behavior

01/10/2010 | ColdFusion | 9 Comments

Today a friend asked me to look over some code he wrote using an MVC framework. One of the first things I noticed was that there was just to much logic going on in some of the views. I would see code that was directing the behavior of the view which is a big no no, we will get to that in a second. In the edit forms for a product I saw logic like this. <cfif request.productId EQ 0> <h2>Add New Product</h2> <cfelse> <h2>Edit #request.product.getProductName()#</h2> </cfif>

You may not realize it but by doing this your actually breaking one of the key concepts of MVC. The view should only concern itself with presentation and it should always look to delegate to the controller for its logic. The reason we do this is to keep modularity by maintaining loose coupling with the controller. A better way of handling the issue above would be to delegate this logic to our controller. component accessors="true" { property ProductDAO; public void function list(){ rc.products = ProductDAO.list(); } public void function edit(){ var id = structKeyExists(rc,"id") ? rc.id : 0; rc.product = ProductDAO.load(id); // page title if(id){ rc.title = rc.product.getProductName(); } else { rc.title = "Add New Author"; } } }

I guess the main point of this entry is to remind us (I make the same mistake) that just because we use a framework doesn't mean your work is done. You still need to be mindful of why you are using it in the first place!

This entry was posted on January 10, 2010 at 11:56 PM and has received 1753 views. Comments 9 | Print this entry.

TweetBacks

Comments (Comment Moderation is enabled. Your comment will not appear until approved.)

[Add Comment] [Subscribe to Comments]

#1 by Raymond Camden on 1/11/10 - 6:16 AM

I'm not so sure I agree with this. I mean, if you take things to the extreme, then your view is simply one pair of cfoutputs and nothing more. To say there should never be _any_ logic in the view isn't really practical. Also, I feel like the View should handle View type issues, which is exactly what your title code was. I mean consider localizations. Your model is now tied to English, it is not?

Now, if you want to get anal about it, I could see adding code to your model: amINew() (horrible method name). Your view could then do:

<cfif product.amINew()>
<cfset title = "Add...">
<cfelse>
<cfset title = "Edit...">
</cfif>
#2 by Brian Swartzfager on 1/11/10 - 7:06 AM

Totally agree with the two points Ray made. It's certainly a good idea to make sure there isn't too much logic in your view pages, and to think about whether some of that logic might be more appropriate to handle in the model, but views should be allowed to have some logic in them when it's appropriate.
#3 by Dan Vega on 1/11/10 - 7:25 AM

I don't agree that this is an extreme case. The page has a title and the logic to display the title is based off of what the model is giving to us. To me that is exactly what the controller is in place for.
#4 by Raymond Camden on 1/11/10 - 7:41 AM

Why would the controller care about a layout issue, which the TITLE tag is? Nor should the model.
#5 by Brian Swartzfager on 1/11/10 - 8:22 AM

Let's look at this a different way...one of the benefits in keeping business/object logic encapsulated in the model is that it allows you to take the model code and drop it into another application without having to modify it, right?

So (to use your example) say you're asked to write another application that uses these Product objects for some other purpose. Your clients for this new application also want to be able to add new products, but they don't want the page title to be "Add New Product" (they're being nitpicky and want something else). That means you'd end up either needing to change your edit() function or (even worse) making a new edit() function in your model just to accomodate this different view.

And if the new application didn't even allow users to add new products (maybe the app is simply a read-only interface that lists products), you'd have this page title code in your model that would simply be taking up space/not being utilized.
#6 by Sam Farmer on 1/11/10 - 9:51 AM

I'm all for dumb views (but not dumb viewpoints!) but to me either way is fine as long as its consistent across the whole project.
#7 by Adam Haskell on 1/11/10 - 3:14 PM

I also agree with Ray Dan ;) The views can talk directly to the model, it just shouldn't reach further back than the controller to get the model.

I do disagree with Ray on the analness of isNew, nothing but the model should have logic to know if it is new or not(unless the lack of an object means it is new <cfif controller.getObject() EQ "" />). Views visually represent the state of object(s). Views talk directly to Objects to interrogate and figure out how to represent the state of the object.
#8 by Raymond Camden on 1/11/10 - 3:18 PM

So to be clear, when you say that you disagree w/ the "anal" part, you mean it is NOT anal and is good practice? If so - yea - anal may have been strong of a term. It's more than I'd do normally though.
#9 by Adam Haskell on 1/11/10 - 3:28 PM

Correct I think it is a good practice not anal. That question (question about newness) is at the core of the state of many objects. So yeah good practice to ask the object about newness.

[Add Comment] [Subscribe to Comments]

Navigation

Search

Tags

Archives by Subject

Subscribe

Recent Comments

Blog Roll

MVC: Delegating interface behavior

TweetBacks

Comments (Comment Moderation is enabled. Your comment will not appear until approved.)