ASP.NET MVC ViewModel Implementation - Is this code best practice?

Question

I just started using ViewModels. Can you guys check this code to see if I follow best practice? Is there anything unusual? Could you do the validation differently?

Sorry if the code is long (there are so many parts). I tried to make it as clear as possible.

Thanks!

Model

public class CustomerModel { [Required(ErrorMessage="Primer nombre!")] public string FirstName { get; set; } [Required(ErrorMessage="Segundo nombre!")] public string LastName { get; set; } [Required(ErrorMessage="Edad")] public int? Age { get; set; } public string State { get; set; } public string CountryID { get; set; } [Required(ErrorMessage="Phone Number")] public string PhoneNumber { get; set; } } 

ViewModel

  public class CustomerViewModel { public CustomerModel Customer { get; set; } public string Phone1a { get; set; } public string Phone1b { get; set; } public string Phone1c { get; set; } } 

controller

  public ActionResult Index() { CustomerViewModel Customer = new CustomerViewModel() { Customer = new CustomerModel(), }; return View(Customer); } [HttpPost] public ActionResult Index(CustomerViewModel c) { //ModelState.Add("Customer.PhoneNumber", ModelState["Phone1a"]); // Let manually bind the phone number fields to the PhoneNumber properties in // Customer object. c.Customer.PhoneNumber = c.Phone1a + c.Phone1b + c.Phone1c; // Let check that it not empty and that it a valid phone number (logic not listed here) if (!String.IsNullOrEmpty(c.Customer.PhoneNumber)) { // Let remove the fact that there was an error! ModelState["Customer.PhoneNumber"].Errors.Clear(); } // Else keep the error there. if (ModelState.IsValid) { Response.Write("<H1 style'background-color:white;color:black'>VALIDATED</H1>"); } return View("Index", c); } } 

View

 @model MVVM1.Models.CustomerViewModel @using (Html.BeginForm("Index", "Detail")) { <table border="1" cellpadding="1" cellspacing="1"> <tr> <td>@Html.LabelFor(m => m.Customer.FirstName)</td> <td> @Html.TextBoxFor(m => m.Customer.FirstName) @Html.ValidationMessageFor(m => m.Customer.FirstName) </td> </tr> <tr> <td>@Html.LabelFor(m => m.Customer.LastName)</td> <td> @Html.TextBoxFor(m => m.Customer.LastName) @Html.ValidationMessageFor(m => m.Customer.LastName) </td> </tr> <tr> <td>@Html.LabelFor(m => m.Customer.Age)</td> <td> @Html.TextBoxFor(m => m.Customer.Age) @Html.ValidationMessageFor(m => m.Customer.Age) </td> </tr> <tr> <td>@Html.LabelFor(m => m.Customer.PhoneNumber)</td> <td width="350"> @Html.TextBoxFor(m => m.Phone1a, new { size="4", maxlength="3" }) @Html.TextBoxFor(m => m.Phone1b) @Html.TextBoxFor(m => m.Phone1c) <div> @Html.ValidationMessageFor(m => m.Customer.PhoneNumber) </div> </td> </tr> <tr> <td></td> <td> <input type="submit" value="Submit" /></td> </tr> </table> } 

Answer 1

One thing that jumps at me is this:

  if (ModelState.IsValid) { Response.Write("<H1 style'background-color:white;color:black'>VALIDATED</H1>"); } return View("Index", c); 

Remember that view models are good for transferring data to your controller and back to your model. I recommend that you add the IsValid property to your view model and then set it to true instead of calling Response.Write. Then just add this to the top of the partial view:

 @if (Model.IsValid) { <H1 style'background-color:white;color:black'>VALIDATED</H1> } 

You can also refer to ModelState, but some argue that this is not the best practice. However, if you do not want to add a property to your model for something that you can just see in your view, you can simply do this:

 @if (ViewData.ModelState.IsValid) 

Another absurdity is that MVC validation attributes are commonly used for validation in the user interface. This check can be reused in other areas, but in some cases is not optimal. In addition, you may not always be able to change your domain models. Therefore, in order to keep all my UI certification in one place, I usually wrap my domain models in my view models so that you get something like this:

 public class CustomerViewModel { public CustomerModel Customer { get; set; } [Required(ErrorMessage="Primer nombre!")] public string FirstName { get { return Customer.FirstName; } set { Customer.FirstName = value; } } ... 

This may seem redundant and not always worth the effort, but when using Entity Framework domain models or other classes that are difficult or impossible to change, good practice should be considered.

Answer 2

I just get MVC freezing, but yesterday I explored this topic and came to the conclusion that the ViewModel should not directly include the model object. Therefore, I understand that it would be bad practice to include your CustomerModel directly in the CustomerViewModel.

Instead, you want to list all the properties from the CustomerModel that you want to include in your ViewModel. Then you either want to manually map the data from CustomerModel to CustomerViewModel or use a tool like AutoMapper that does this automatically with the same line of code inside your action:

 public ViewResult Example() { // Populate/retrieve yourCustomer here Customer yourCustomer = new CustomerModel(); var model = Mapper.Map<CustomerModel, CustomerViewModel>(yourCustomer); return View(model); } 

In this case, Mapper.Map will return a CustomerViewModel, which you can pass into your view.

You also need to include the following in your Application_Start method:

 Mapper.CreateMap<CustomerModel, CustomerViewModel>(); 

In general, I found that AutoMapper is pretty easy to work with. It automatically if the field names match, if they do not exist or you have a nested object, you can specify these mappings in the CreateMap line. Therefore, if your CustomerModel uses the Address object instead of the individual properties, you should do this:

 Mapper.CreateMap<CustomerModel, CustomerViewModel>() .ForMember(dest => dest.StreetAddress, opt => opt.MapFrom(src => src.Address.Street)); 

Please someone correct me if I am wrong as I just get the head from MVC.

Answer 3

I would say that your ViewModel implementation is pretty standard. You use the ViewModel to act as an intermediate object between your view and your domain model. This is a good practice.

The only thing I'd be tired of about is how you handle model errors, and also your ViewModel should have some attributes. For example, you can use the RegularExpressionAttribute class:

  public class CustomerViewModel { public CustomerModel Customer { get; set; } [RegularExpression(@"^\d{3}$")] public string Phone1a { get; set; } [RegularExpression(@"^\d{3}$")] public string Phone1b { get; set; } [RegularExpression(@"^\d{4}$")] public string Phone1c { get; set; } }