3

I want to post a comment to this question's accepted answer, "Haven't views abandoned code behind now? So what are you going to test?" pointing out that it seems to me as soon as you add an 

<% if (Model.Thing == "abc") {} %>

or 

@if (Model.Thing == "abc") {}

to your view, there exists the potential for something to blow up, and that potential should be guarded against.

In regard to the question I linked to, I could see an argument being made that one should guard against the possibility of null reference exceptions in the code-behind rather than littering one's view with null checks, but then how about the case of partial views? Would it really be better to add multiple null checks in the numerous places where a partial view might be rendered, rather than one place, in the view itself?

Community
  • 1
  • 1

3 Answers3

2

IMO, you should only guard against null, index oob, etc, in your views if you are expecting values to be null, oob, etc.

Ideally you should have unit tests for your action methods that ensure certain model values are not null / in bounds / etc. When there is a possibility that a value will be null, then you might have a good reason for a null check in a view. Otherwise, it's useless code.

danludwig
  • 46,965
  • 25
  • 159
  • 237
  • Thanks for the feedback, @olivehour. I think accepting your guidelines will require a change of habit that will be beneficial. It's so tempting and easy to add a guard against a null condition (oob, etc.) right where it occurs in a view, but the answers I'm receiving on here are very consistent, and stand to reason. –  Feb 22 '12 at 16:29
  • Once you start writing unit tests, those habits will be reinforced. You will have unit tests that assert scenarios when a certain piece is/isn't null/oob, etc. If you ever find that something was null when you did not expect it to be null, you are missing a unit test. – danludwig Feb 22 '12 at 16:38
  • I already heart unit tests in a big way. I'll be sure and add this type of check to my repertoire. –  Feb 22 '12 at 17:15
2

Try to get away from the Webforms code-behind way of thinking. The null checks for Model data should be handled in the Controller. The View should contain minimal (or no) logic for that sort of check.

public ActionResult YourAction(YourModel ym)
{
    if (ym.Thing != null)
        return View(ym);
    else
        return View();
}

Or whatever type of check you had to do on the data. That way in your View, it wouldn't be littered with checking Model data. It's all part of the separation of concerns in the MVC design.

  • That won't compile, and I think he's really asking whether he should be checking whether the `Model` is null before using it's properties. – tvanfosson Feb 22 '12 at 15:42
  • @tvanfosson Thanks, fixed. Also, that logic can easily be placed within the Controller code. I just think the View shouldn't be so concerned with that. –  Feb 22 '12 at 15:44
  • I started out on MVC and wouldn't know Webforms if it bit me in the butt, but your advice still stands. Thanks! –  Feb 22 '12 at 15:46
  • @Shark - I would agree that generally the Model should never be null. Unit tests should guarantee this and we really shouldn't have to concern ourselves with it in the view. Your update, though, doesn't demonstrate this. The second return will result in a null Model in the view and would force the view to account for the possibility. – tvanfosson Feb 22 '12 at 15:47
  • @tvanfosson I agree, the Action method I have posted most likely doesn't fit what the user wants to check. It was merely used to show an example of a conditional check you could do in a Controller as opposed to the View. –  Feb 22 '12 at 15:49
1

If there are no legal conditions under which the model or the model's properties should be null, then I would say that your unit tests, not your views should enforce this. If there are cases where the model can be null or contain null properties, then by all means check for this IF the check is so that you can adjust the display. You still need to guard against business logic, as opposed to display logic (some of which may be related to business rules), leaking into your views.

tvanfosson
  • 524,688
  • 99
  • 697
  • 795