An important concept in MVC is that Views are meant to be “dumb” – and by that I mean, your View should only care about outputting a user interface, and should know very little about business logic.
You want this for a few reasons. For example, you might create alternate Views for different kinds of output, such as for a mobile device. You shouldn’t have to re-implement any business logic to do that.
And down the road, when your logic inevitably changes, you shouldn’t have to worry about changing it in more than one place. Not only is that tedious and time-consuming, but you open up the risk of having inconsistent behavior. It violates DRY.
How might this happen? Here’s an example.
Let’s say you have a social app which displays user profiles. If a user is looking at their own profile, you might want to offer them a link to edit it. So your View would have something like:
<% if (Model.DisplayedUser == Model.LoggedInUser) { %>
<a href="url_of_edit_screen">edit this profile</a>
<% } %>
Simple enough, and it might not raise any red flags at the time. Then, however, you realize that administrators should be able to edit profiles as well. Now your View code becomes:
<% if (Model.DisplayedUser == Model.LoggedInUser || Request.User.IsInRole("Administrator")) { %>
<a href="url_of_edit_screen">edit this profile</a>
<% } %>
See where this is going? A month later, you realize that “moderators” need the same ability. And if a profile has been suspended, perhaps the user should no longer see that link. And maybe you want to offer a “preview” mode, where the link is hidden. Etc.
Your View is now handling business logic. It becomes unreadable and untestable, and you are blurring the separation of concerns.
The best way to avoid this is, first, have a ViewModel (the above code already assumes that). Second, add properties to that ViewModel that describe only what the View should show, without regard for why. The usage becomes:
<% if (Model.EnableEditLink) { %>
<a href="url_of_edit_screen">edit this profile</a>
<% } %>
Now, your View doesn’t know anything about Users, Roles or anything like that. It’s simply following orders that describe what to output. For Views, ignorance is bliss.
Meanwhile, back in your ViewModel class, the property might be something like:
public class ProfileViewModel
{
public bool EnableEditLink {
get
{
bool display = (this.DisplayedUser == this.LoggedInUser)
|| Request.User.IsInRole("Administrator")
&& !UserAccountIsSuspended
&& !IsPreviewMode;
return display;
}
}
}
(Another practical benefit: you can’t set a breakpoint in a View. By encapsulating the logic in the ViewModel, you can debug and write tests to your heart’s content.)
So next time you need to put an if-then in your View, resist the urge to let the View make the decision. No matter how simple, push the logic back into the ViewModel, where it belongs.
I was just wondering, shouldn’t the ProfileViewModel object be a container and the business logic be manipulated in your Controller?
As it stands the business logic would be spread across both your Controller and the ViewModel.
I’d go one step further than Adrian actually, just as the view is dumb, the controller should also be dumb. I’d put the logic in another model (could be a domain model, could be a reporting model, could be a security model) and do the mapping in the controller.
To give you an example as to why this is a good idea, think about the scenario where you have more than one "edit" link on different screens. Your logic is encapsulated in the ViewModel for *one* screen when really it is likely a more general concern.
I typically keep ViewModels nearly as dumb as Views.
Actually you CAN set a breakpoint in the view , just have the cursor somewhere in the if statement and hit F9 , it should work.
But your point is still valid
I’m not even sure if that boolean check should be part of the View Model. I think that the View Model should contain mostly read-only fields and some formatting logic (i.e. "Negative Numbers are displayed with css-class xyz"), but I’d rather but the "EnableLink?" Logic in the controller that creates the ViewModel and passes it into the view.
But generally, I strongly agree. The View should just output whatever is in the viewmodel and not ask any questions about the why.
Good post.
I also did a blog post on how to keep logic out of your views which shows how you can refactor if’s in your views.
http://www.arrangeactassert.com/asp-net-mvc-view-best-practices-keep-logic-out-of-your-views
Cheers,
Jag
In your example, how are you accessing the Request object? I would have thought that the EnableEditLink method would work better as a HtmlHelper, then it’s not restricted to a single ViewModel.
Rich
That’s a new one for me!
I would strongly advise against putting things like this in a Helper. The Helper is a Helper, it should be just as dumb as the view. It’s a slippery slope between stuff like this and just accessing your db in the helper. Like I said, these things should be done in the layer that the controller accesses and passes back to the view as a flag.
Your penultimate sentence: <<So next time you need to put an if-then in your View>> I think should read: …to put in multiple if-then…
Your "good" example still has an "if" statement in it.
I forgot to mention one of the most important reasons to not put this in your view model or a helper.
I assume you want to do this same check in the actual "Edit" action, right? I mean, a hidden link can still be clicked if it’s emailed to someone. Putting the logic in the view model or an html helper makes it rather awkward to use within the controller itself.
Fair enough! It can be encapsulated further.
That’s true, it’s part of the greater issue that one shouldn’t depend on anything client-side in terms of validation. The "Edit" method behind that link needs to have similar logic.
Regards for this tremendous post, I’m glad I noticed this internet site on yahoo.