17

I'm relatively new to view models and I'm running into a few problems with using them. Here's one situation where I'm wondering what the best practice is...

I'm putting all the information a view needs into the view model. Here's an example -- please forgive any errors, this is coded off the top of my head.

public ActionResult Edit(int id)
{
    var project = ProjectService.GetProject(id);

    if (project == null)
        // Something about not found, possibly a redirect to 404.

    var model = new ProjectEdit();
    model.MapFrom(project); // Extension method using AutoMapper.

    return View(model);
}

If the screen only allows editing of one or two fields, when the view model comes back it's missing quite a bit of data (as it should be).

[HttpPost]
public ActionResult Edit(int id, ProjectEdit model)
{
    var project = ProjectService.GetProject(id);

    if (project == null)
        // Something about not found, possibly a redirect to 404.

    try
    {
        if (!ModelState.IsValid)
            return View(model) // Won't work, view model is incomplete.

        model.MapTo(project); // Extension method using AutoMapper.
        ProjectService.UpdateProject(project);
        // Add a message for the user to temp data.

        return RedirectToAction("details", new { project.Id });
    }
    catch (Exception exception)
    {
        // Add a message for the user to temp data.

        return View(model) // Won't work, view model is incomplete.
    }
}

My temporary solution is to recreate the view model from scratch, repopulate it from the domain model, reapply the form data to it, then proceed as normal. But this makes the view model parameter somewhat pointless.

[HttpPost]
public ActionResult Edit(int id, ProjectEdit model)
{
    var project = ProjectService.GetProject(id);

    if (project == null)
        // Something about not found, possibly a redirect to 404.

    // Recreate the view model from scratch.
    model = new ProjectEdit();
    model.MapFrom(project); // Extension method using AutoMapper.

    try
    {
        TryUpdateModel(model); // Reapply the form data.

        if (!ModelState.IsValid)
            return View(model) // View model is complete this time.

        model.MapTo(project); // Extension method using AutoMapper.
        ProjectService.UpdateProject(project);
        // Add a message for the user to temp data.

        return RedirectToAction("details", new { project.Id });
    }
    catch (Exception exception)
    {
        // Add a message for the user to temp data.

        return View(model) // View model is complete this time.
    }
}

Is there a more elegant way?

EDIT

Both answers are correct so I would award them both if I could. The nod goes to MJ however since after trial and error I find his solution to be the leanest.

I'm still able to use the helpers too, Jimmy. If I add what I need to be displayed to the view bag (or view data), like so...

ViewBag.Project= project;

I can then do the following...

@Html.LabelFor(model => ((Project)ViewData["Project"]).Name)
@Html.DisplayFor(model => ((Project)ViewData["Project"]).Name)

A bit of a hack, and it requires the domain model to be decorated with System.ComponentModel.DisplayNameAttribute in some cases, but I already do that.

I'd love to call...

@Html.LabelFor(model => ViewBag.Project.Name)

But dynamic causes a problem in expressions.

fretje
  • 8,322
  • 2
  • 49
  • 61
Adam Boddington
  • 6,750
  • 2
  • 21
  • 12
  • 1
    you can look here : http://prodinner.codeplex.com/ for best-practices (including viewmodels) in asp.net mvc – Omu Mar 16 '11 at 20:24

2 Answers2

13

After some trial-and-error (aka code it, then hate it) learning, my currently preferred approach is:

I use view-models only to bind input fields. So in your case, if your view is only editing two fields, then your view-model will only have two properties. For the data required to populate the view (drop-down lists, labels, etc), I use the dynamic ViewBag.

I believe that displaying the view (i.e. populating anything the view needs to display), and capturing the posted form values (binding, validation, etc) are two separate concerns. And I find that mixing the data required to populate the view with that which is posted back from the view gets messy, and creates exactly your situation more often than not. I dislike partially populated objects being passed around.

I’m not sure how this plays out with Automapper (for mapping the domain object to the dynamic ViewBag) though, as I haven’t used it. I believe it has a DynamicMap method that may work? You shouldn’t have any issues auto-mapping the posted strongly-typed ViewModel onto the Domain object.

MJ Richardson
  • 1,441
  • 12
  • 30
  • 1
    Ignoring AutoMapper - that's only if you have the problem of mapping objects, display-only view models are really helpful if you're doing the MVC templated helpers (Html.DisplayFor). – Jimmy Bogard Mar 15 '11 at 03:13
  • I like the sound of this because my view models would reflect only what I want back and would become leaner as a result. I would lose `Html.DisplayFor` as Jimmy says, but that could be remedied with some `HtmlHelper` extensions of my own. I'm not enjoying the view model "tax" at the moment -- this approach has the advantage of reducing it somewhat. – Adam Boddington Mar 15 '11 at 03:48
8

If I understand correctly, your viewmodel probably looks very similar to your domain entity. You mentioned that the viewmodel can come back mostly empty because only certain fields were editable.

Assuming you have a view where only a few fields are available for edit (or display), these are the only fields you should make available in your viewmodel. I usually create one viewmodel per view, and let either the controller or a service handle the user's input and map it back up with the domain entity after performing some validation.

Here's a thread concerning best-practices for viewmodels that you might find useful.

Edit: You can also accept a different view model in your Edit/POST action than your Edit/GET action serves up. I believe this should work as long as the model binder can figure it out.

Community
  • 1
  • 1
Andrew Whitaker
  • 124,656
  • 32
  • 289
  • 307
  • That has the added advantage of making my view models much leaner. But where does the display only data go? Do you put domain model objects (or DTOs) in ViewData/ViewBag to handle that? – Adam Boddington Mar 15 '11 at 02:12
  • @Adam: I should probably re-state my answer. Data you're displaying should be included in your view models as well. – Andrew Whitaker Mar 15 '11 at 02:32
  • @Adam: See my updated answer. Your edit/POST action could contain only the editable fields – Andrew Whitaker Mar 15 '11 at 03:12
  • So that means the GET view model needs to be recreated (and have the form data applied to it if I want the user to see what they changed) before showing the view again in the event of invalid data or an exception. Similar to the last action method in my question, but perhaps not recreating the GET view model until absolutely needed. – Adam Boddington Mar 15 '11 at 03:39
  • @Adam: Yes, that's correct. Additionally, you would have to recreate the GET view model when server-side validation fails. However, tools like AutoMapper make this easier. – Andrew Whitaker Mar 15 '11 at 11:40
  • I've been trying this out, Andrew, and there seems to be a security risk. By having everything in the view model, editable data as well as non-editable data, couldn't a user post back extra values as if the non-editable data was actually editable? The controller wouldn't know the difference because it's all in the view model, and so would accept it. There is a case I think here for only having what is editable in the view model, and putting everything else in the view bag, ala Michael's answer. Thoughts? – Adam Boddington Mar 20 '11 at 22:19
  • I guess this is where another view model is used for the post action which contains only the values that are expected back. But then I'm dealing with two view models. – Adam Boddington Mar 20 '11 at 22:29