Null checks in view

Question

It's really a nitty-gritty issue for me, and I have been here many times before. How could this be improved?

I have a basic viewmodel for a view:

public class LoginViewModel : BaseViewModel
{
 public User _User { get; set; }
}

My base class looks like this:

public class BaseViewModel
{
 public List<string> StateMessages { get; set; }
}

The view takes a LoginViewModel, but if I do not pass it in from the controller, then I need to check the StateMessages for null before I access it. So I pass in an empty ViewModel, which is okay, but if I could get around it, it would just look better.

[Route("login")]
[Route( "~/", Name="default" )]
public ActionResult Login()
{
 if (User.Identity.IsAuthenticated)
 return RedirectToAction("Index", "Dashboard");
 return View(new LoginViewModel());
}

Then in the view I don't have check for null, which I prefer:

@foreach (var err in Model.StateMessages)
{
 <span>@err</span><br />
}

I know this is very cosmetic, but is this just they way it should be, or can I somehow avoid to new up a new and empty ViewModel in the controller?

• Heslacher: Please add more code to get a better view of your problem. Where do you do the null check ? Where and how do you pass an empty ViewModel ? I have deleted my answer so it is legit to add more code.
• user3633222: At the moment I don't do a null check anywhere. And I want to keep it than way. But I believe passing in an empty viewmodel, just to come around it, is equally a bad design. But there might not be a way around it. Ideally I would like just "return View();" and in the view, still do the above. But since the model would be null, I need to do a null check in the view, which is just ugly.
• Ben Aaronson: I don't see a problem with the empty viewmodel. It seems to better represent the situation. You have no state messages, that corresponds to an empty collection of state messages, not to a null collection.
• user3633222: Thx Ben, that was actually all I needed to hear :)

Answer 1

I would suggest changing the type of StateMessages from List<string> to IEnumerable<string>.

It hides implementation details, allowing you flexibility in your choice of data structure in the future.

It also gives you more control over how client code can access/modify the state messages.

Make the backing field readonly to ensure that it's not null.

public class BaseViewModel
{
 private readonly List<string> stateMessages = new List<string>();
 public IEnumerable<string> StateMessages
 {
 get { return this.stateMessage; }
 }
 // Other methods to access/modify stateMessages.
}