When is my View too smart?
- by Kyle Burns
In this posting, I will discuss the motivation behind
keeping View code as thin as possible when using patterns such as MVC, MVVM,
and MVP. Once the motivation is
identified, I will examine some ways to determine whether a View contains logic
that belongs in another part of the application. While the concepts that I will discuss are
applicable to most any pattern which favors a thin View, any concrete examples
that I present will center on ASP.NET MVC.
Design patterns that include a Model, a View, and other
components such as a Controller, ViewModel, or Presenter are not new to
application development. These patterns
have, in fact, been around since the early days of building applications with
graphical interfaces. The reason that
these patterns emerged is simple – the code running closest to the user tends
to be littered with logic and library calls that center around implementation
details of showing and manipulating user interface widgets and when this type
of code is interspersed with application domain logic it becomes difficult to
understand and much more difficult to adequately test. By removing domain logic from the View, we
ensure that the View has a single responsibility of drawing the screen which,
in turn, makes our application easier to understand and maintain.
I was recently asked to take a look at an ASP.NET MVC View
because the developer reviewing it thought that it possibly had too much going
on in the view. I looked at the .CSHTML
file and the first thing that occurred to me was that it began with 40 lines of
code declaring member variables and performing the necessary calculations to
populate these variables, which were later either output directly to the page
or used to control some conditional rendering action (such as adding a class
name to an HTML element or not rendering another element at all). This exhibited both of what I consider the
primary heuristics (or code
smells) indicating that the View is too smart:
Member variables – in general, variables in View
code are an indication that the Model to which the View is being bound is not
sufficient for the needs of the View and that the View has had to augment that
Model. Notable exceptions to this
guideline include variables used to hold information specifically related to
rendering (such as a dynamically determined CSS class name or the depth within
a recursive structure for indentation purposes) and variables which are used to
facilitate looping through collections while binding.
Arithmetic – as with member variables, the
presence of arithmetic operators within View code are an indication that the
Model servicing the View is insufficient for its needs. For example, if the Model represents a line
item in a sales order, it might seem perfectly natural to “normalize” the Model
by storing the quantity and unit price in the Model and multiply these within
the View to show the line total. While
this does seem natural, it introduces a business rule to the View code and
makes it impossible to test that the rounding of the result meets the
requirement of the business without executing the View. Within View code, arithmetic should only be
used for activities such as incrementing loop counters and calculating element
widths.
In addition to the two characteristics of a “Smart View” that
I’ve discussed already, this View also exhibited another heuristic that
commonly indicates to me the need to refactor a View and make it a bit less
smart. That characteristic is the
existence of Boolean logic that either does not work directly with properties
of the Model or works with too many properties of the Model. Consider the following code and consider how logic
that does not work directly with properties of the Model is just another form
of the “member variable” heuristic covered earlier:
@if(DateTime.Now.Hour < 12) {
<div>Good Morning!</div>
} else {
<div>Greetings</div>
}
This code performs business logic to determine whether it is
morning. A possible refactoring would be
to add an IsMorning property to the Model, but in this particular case there is
enough similarity between the branches that the entire branching structure
could be collapsed by adding a Greeting property to the Model and using it
similarly to the following:
<div>@Model.Greeting</div>
Now let’s look at some complex logic around multiple Model
properties:
@if (ModelPageNumber + Model.NumbersToDisplay == Model.PageCount
|| (Model.PageCount !=
Model.CurrentPage
&&
!Model.DisplayValues.Contains(Model.PageCount))) {
<div>There's more to
see!</div>
}
In this scenario, not only is the View code difficult to
read (you shouldn’t have to play “human compiler” to determine the purpose of
the code), but it also complex enough to be at risk for logical errors that
cannot be detected without executing the View.
Conditional logic that requires more than a single logical operator should
be looked at more closely to determine whether the condition should be
evaluated elsewhere and exposed as a single property of the Model. Moving the logic above outside of the View
and exposing a new Model property would simplify the View code to:
@if(Model.HasMoreToSee) {
<div>There’s more to
see!</div>
}
In this posting I have briefly discussed some of the more
prominent heuristics that indicate a need to push code from the View into other
pieces of the application. You should
now be able to recognize these symptoms when building or maintaining Views (or
the Models that support them) in your applications.
