6

All too often I've run into a situation in which a view in my project throws a null reference exception. 

@model Johnny.Application.TestModel
<div>@(Model.SomeText)</div>

This throws an error if Model is null.

But how are people handling this? I certainly don't see code samples everywhere with ugly null checks littering the code in the view. That leads me to believe that most of the time, controllers aren't supposed to return null models. But how can you enforce this with more finesse?

Right now as soon as someone accidentally causes a controller to return a null model, the view model blows up and looks to be at fault. In reality, it was the controller's fault. And the view may not even "catch" the problem, it will only do so if the model's members happen to get used (which is most of the time, of course).

For various reasons, some views may want to handle null values. I wouldn't expect this to be the majority case, though. Clearly this is the matter of setting some "contract" between view and controller.

I don't like the options I've seen:

  1. Check if model is null every time it's used. Very lame!
  2. One big if statement wrapping the whole view with a null model check. Think of the wasted code real estate. Lame!
  3. Add an if check with a throw at the top. Not bad, but seems silly. Mildly lame.

I would love to know if something like these options existed to set the "no nulls" contract:

  • An attribute on the controller method like [NoNullModels]. I doubt this exists, since I don't think the controller knows what view it is hooking up to.
  • In the view, an indicator like @MVC3.HeyDontAllowNulls or some other standard way of throwing an exception (like option 3 above)
Johnny Kauffman
  • 3,613
  • 3
  • 21
  • 22
  • why would you even return a null model? – Daniel A. White Mar 01 '12 at 15:18
  • 1
    have you tried `@Html.DisplayFor(m => m.SomeText)` – Daniel A. White Mar 01 '12 at 15:20
  • On the code sample topic, 99% of code samples on the Internet are devoid of exception handling and input validation. Partly out of laziness and partly because it would confuse the point that the code sample is illustrating. – Adrian Thompson Phillips Mar 01 '12 at 15:36
  • I'd also look at using some sort of global exception logging, such as ELMAH and making sure that you have an friendly error page to show users when something blows up. You can then keep an eye on the ELMAH log and fix critical errors that your code doesn't anticipate. – Adrian Thompson Phillips Mar 01 '12 at 15:38
  • Given that it is the controller's job to pass the model to the view shouldn't the handling of the null model reside there rather than the view? Complicating the view because the controller isn't keeping its side of the bargain doesn't seem like the right approach. – Jack Hughes Mar 01 '12 at 15:21
  • I am aware the controller is at fault. My situation is analogous to writing a function that throws "ArgumentNullException", which appropriately identifies the source of the blame. In fact, that may even be the best thing to do here! – Johnny Kauffman Mar 01 '12 at 16:53

3 Answers3

2

I asked a similar question here Should one try to guard against null reference exceptions / index out of bounds exceptions in MVC views? and got good responses to it. In short, it is preferred to add null checks in your controllers and perhaps even unit tests rather than in your views.

Community
  • 1
  • 1
  • I hadn't thought of the unit testing approach, and this is a good scenario for their utility. Since I don't "own" all the code, I won't be able to make all controllers unit testable, so I'm hoping to find a fallback option for the view in these cases. – Johnny Kauffman Mar 01 '12 at 15:58
  • I also admit that if I had, say, a highly-shared view which got rendered from numerous action methods, I would sure be tempted to add a null check in one place, in the view itself, rather than in a dozen other places even if I did own all of the code. You might be able to call that an adherence to the DRY principle at the cost of violating the MVC Separation of Concerns a bit. –  Mar 01 '12 at 16:06
  • The more I think about it, the more I feel that the view is the right place to add something. One reason is that a single controller action can route to several different views. Fixing it on the controller side isn't effective, since it'll spring a leak sooner or later (like the situation I keep running into!) – Johnny Kauffman Mar 01 '12 at 16:54
0

There are many preferences here, some that you can do are:

  1. Throw RecordNotFoundException in your data layer (custom exception) and have a global exception filter catch this
  2. Check for nulls coming back from your repository and handle in a case by case scenario
  3. Decorate your controllers to look for a null model on GET methods that aren't CREATE methods (or whatever rules you deem OK) with an action filter attribute to check for this in OnActionExecuted

Even my "CREATE" views generally get a model, even if empty (although quite often have a IEnumerable<SelectListItem> in them), so they should always have model going to them.

Adam Tuliper
  • 29,982
  • 4
  • 53
  • 71
0

I check for nulls in my views sometimes in parts of it that require it.

I also will sometimes create default values in my controller if there is going to be a null value depends on what you are trying to do and what is acceptable.

I have for instance a case where people subscribe to something and set notifications. If they have no notifications a sub-object of my model is null. I don't want to set default values so I have a check there. In other sections i just use a default value.

Jordan
  • 2,708
  • 4
  • 22
  • 35