How can I avoid using a ViewBag in MVC4 to pass query values between controller and view

Question

I've read that it's generally a bad practice to use ViewBags and also bad to mix ViewBag and ViewModels, but I'm using some ViewBags to pass sort order parameters between my controller and view and though I can create the properties on the ViewModel for the view I need the values before I can create the model in the first place, so can't see how to get around this.

Controller

public ActionResult Index(string sortOrder, string searchString, 
                          string currentFilter, int? page, 
                          bool? includeComplete)
{

 ViewBag.CurrentSort = sortOrder;
 ViewBag.NameSortParam = string.IsNullOrWhiteSpace(sortOrder) ? "Name desc" : "";
 ViewBag.DateSortParam = sortOrder == "Date" ? "Date desc" : "Date";

 if (Request.HttpMethod == "GET")
     searchString = currentFilter;
 else
     page = 1;

 ViewBag.CurrentFilter = searchString;
 bool showCompleted = (includeComplete == null || includeComplete == false)
                        ? false : true;
 ViewBag.IncludeCompleted = showCompleted;

 int pageNumber = (page ?? 1);

 var query = Session.QueryOver<ToDo>();

 if (!string.IsNullOrWhiteSpace(searchString))
     query = query.WhereRestrictionOn(td => td.TaskName)
                      .IsInsensitiveLike(string.Format("%{0}%", searchString));

 if (!showCompleted)
     query.And(td => td.IsComplete == false);

 switch (sortOrder)
 {
    case "Name desc":
      query = query.OrderBy(td => td.TaskName).Desc;
      break;
        case "Date":
      query = query.OrderBy(td => td.DueDate).Asc;
      break;
    case "Date desc":
      query = query.OrderBy(td => td.DueDate).Desc;
      break;
    default:
      query = query.OrderBy(td => td.TaskName).Asc;
      break;
 }

 var result = query.Fetch(p=>p.Priority).Eager
           .Fetch(s=>s.Staff).Eager
           .List();

 var viewModel = AutoMapper.Mapper.Map<IEnumerable<ToDo>, 
                     IEnumerable<IndexToDoViewModel>>(result)
                     .ToPagedList(pageNumber, PageSize);
 return View(viewModel);

}

Then in my view I'll use the ViewBag to pass values back to the controller, such as the sortOrder which I need to have before I can apply it in my NHibernate query that forms the bulk of the method, for example:

@Html.ActionLink("Name", "Index", new {sortOrder=ViewBag.NameSortParam, 
                                      includeComplete = ViewBag.IncludeCompleted})

My ViewModel

public class IndexToDoViewModel
{
    [DataType(DataType.Date)]
    [DisplayFormat(DataFormatString = "{0:dd MMM yyyy}")]
    [DisplayName("Date Due")]
    public DateTime? DueDate { get; set; }

    public Guid Id { get; set; }

    [DisplayName("Is Complete")]
    public bool IsComplete { get; set; }

    [DataType(DataType.MultilineText)]
    public String Notes { get; set; }

    public string Priority { get; set; }

    public string Staff {get; set;}

    [DisplayName("Name")]
    public String TaskName { get; set; }

    // These would potentially replace my ViewBag?
    public string CurrentSort { get; set; }
    public string CurrentFilter { get; set; }
    public string NameSortParam { get; set; }
    public string DateSortParam { get; set; }
    public bool IncludeCompleted { get; set; }

}

Why not include SortOrder in your ViewModel? – Oliver Mar 17 '13 at 23:07 — Oliver, Mar 17 '13 at 23:07

Guffa · Accepted Answer · 2013-03-17T23:30:57.090

3

Then in my view I'll use the ViewBag to pass values back to the controller

No, you are passing the values to the next controller. The values will (potentially) be used after a round trip to the browser, when a new instance of the controller will handle the request.

As you are not passing anything back to the current controller, there is no need to use the ViewBag. You can put the values in the model if you like.

Even if you were sending data from the view to the controller (which is very unusual), that would still be possible using the model. Example:

MyModel viewModel = new MyModel();
ActionResult result = View(viewModel);

// here you can access anything that the view would put in the model

return result;

Of course, you can't use any values from the view in the controller to create the model, as you have to create the model before the view. If you would need any data from the view to get data for it, you would put that as a method in the model instead of in the controller.

edited Mar 17 '13 at 23:30

answered Mar 17 '13 at 23:25

Guffa

687,336
108
737
1,005

I'm unclear how that would work; for example in the `ActionLink` code it's getting the "Name" (display value), "Index" (controller) and then the other properties are set... How can I set those properties on my ViewModel? – Simon Martin Mar 17 '13 at 23:29
@SimonMartin: You can put anything in your model. Currently you have a `Map` object as model, instead you would create a custom object that has the `Map` object as a property, and any other data that you need as properties as well. – Guffa Mar 17 '13 at 23:33
But how does the view send the model to the controller with the value, in the example of my "Name" sort, what would go in my `ActionLink` after the display value and method name - where I've currently got the ViewBag properties being set? – Simon Martin Mar 17 '13 at 23:46
1

@SimonMartin: The view is not sending the model to the controller, it's the other way around. What's in the `ActionLink` goes into a URL, that is used to make another request, which will end up in a controller in the future, not the controller used to create this response. You would use whatever values you want in the `ActionLink` depending on how you want that future controller to sort the data. If you want that sorted just like the data in the current response, you would use the same values as was used to get the data for the current model. – Guffa Mar 18 '13 at 00:15
Right; I *think* I've got it. The properties in my ViewModel can be accessed something like this `@Html.TextBox("SearchFilter", Model.SearchFilter)` and the `ActionResult` arguments becomes just `IndexToDoViewModel` the `logic` is then removed from the Views and exists in the Controller and Model. However as @Oliver says I'll need to abstract up a layer and wrap my current ViewModel as a property of one which exposes it as an `IEnumerable` in order to make the PagedList interface happy. – Simon Martin Mar 18 '13 at 00:25
@SimonMartin: Yes, you can use the model as parameter in the action method, and the properties would be populated from the route, but you can also use simple parameters as you do now, and populate the model with them yourself. – Guffa Mar 18 '13 at 00:41

score 1 · Answer 2 · edited May 23 '17 at 12:28

1

I would recommend creating a ViewModel for each of the views you create as it is most likely that further down the line you will need to pass more information to your view.

In this case, you would have a ViewModel that has a property for the paged list data and you can add another property for the SortOrder.

For example:

public class MyViewModel
{
    public IEnumerable<ToDo> TodoList { get; set; }
    public string SortOrder { get; set; }
}

See SO question: ViewModel Best Practices

edited May 23 '17 at 12:28

Community

1
1

answered Mar 17 '13 at 23:08

Oliver

8,794
2
40
60

That's what I was thinking, but I need the `SortOrder` property available right at the top of the controller, but it would only be available to me at the bottom of the controller after I've initialised the viewModel right before returning it to the View. – Simon Martin Mar 17 '13 at 23:25
@SimonMartin: No, the sort order in the model would just be the value that you put in it. To sort the data for this page you want the sort order from the *previous* page, that you pick up from the URL for the request, i.e. the `sortOrder` parameter in your action method. – Guffa Mar 17 '13 at 23:35

How can I avoid using a ViewBag in MVC4 to pass query values between controller and view

2 Answers2