A lot of people seem to have an allergic reaction towards anything not “strongly typed”. They abhor ViewBag and resent ViewData[“key”] for passing values from the controller to the view. They write viewmodels that aggregate a few other viewmodels or worse, use inheritance to generate complex hierarchies. And in order to try and keep all that clean, they use design patterns and what not to achieve “compile time checking”, “testability” and a few other mirages of “good code”. Maybe I like leaning over the edge a bit, but I really can’t understand why something that is to be a data carrier has to be part of something that’s as complex (if not more) than a trivial application’s domain. Here are some of the drawbacks of strongly typing all viewmodel data:
Hideous hideous chaining
If you have viewmodel classes that encapsulate other viewmodel classes, your view will like have code like:
Model.MemberAddressVM.X.Y.WhatNot
Granted the effects can be mitigated by using partials, RenderAction and other stuff; and this is not the main problem.
Using hierarchies
If you use a base viewmodel class and set some default stuff further up the hierarchy (in an ApplicationController base class or something), you won’t get (much of) the chaining problem, but you will tightly couple the behaviour for all controllers that share the same base. In many cases, the viewmodel data is only for the specific view and meaningless for the others. Anytime we favour inheritance, we should stop and think whether the coupling is worth it.
Extra burden
Right, so if we choose the aggregated viewmodel approach, we take on a great deal of responsibility. We need to create the wrapper class, pass in parameters via the ctor, do the newing up in the controller, write tests to verify…while sometimes all of that can be useful, in many cases it’s not. Consider this example:
A member has many addresses. The /member/rooney/addresses page has a list of addresses and a button named “create new”. Clicking the button shows a jquery iframe popup which handles the create form. The form needs to know about the member’s id (to know which member the new address would belong to). The grid showing all addresses for the user needs an IEnumerable<Address>. So just to pass in MemberId to the addresses page, (if we were using an aggregate viewmodel) our viewmodel would be like this:
MemberAddressesViewModel
- MemberId
- IEnumerable<MemberAddress>
or even
MemberAddressesViewModel
- MemberId
- MemberAddressGridViewModel
where the latter would just wrap an IEnumerable<MemberAddress>.
We would then have controller code like:
var gridItems = db.GetAll<Address>().Where(x=>x.MemberId == memberId);
var model = new MemberAddressesViewModel(memberId, new MemberAddressGridViewModel(gridItems));
return View(model);
And I won’t bother writing the test for that.
False sense of doing it right
Many think that only using strongly typed vms is the “right” and “proper” way of doing things. They assume that since it’s strongly typed and “driven” by intellisense, it must be right. This is far from the case…at best it just gives you syntactic validation. If you accept that as enough, you run the risk of having semantic errors. If MemberId and LegacyMemberId were both present in the viewmodel and somebody used MemberId to create a link to the member’s legacy data, chances are that’s still wrong, although intellisense and view compilation would let that pass. Even with all those additional indirections, something as simple as which Id should be used go through. When using ViewBag, it’s quite likely that you would double check if you’re using the right thing as it’s loosely typed and you know it can be prone to errors. When using the strongly typed version, after having dedicated vm code, tests to cover it and seeing it pass intellisense, you might accept that simply using the MemberId to generate a link on a member details page may be enough. This shouldn’t be the case and you should be doing UI testing – but if you’re doing that, any errors with the ViewBag version would also get caught. So apart from seeing the cool intellisense, are you really gaining any benefit?
But ViewBag isn’t statically checked…that’s bad, right?
Is it? Well, I can understand where lack of static checking might be alarming (I still like C# over ruby), but there are cases where the ceremony needed to achieve the static checking means a lot more work with very little gain. In the previous example, We could have left the index page’s model to be of type IEnumerable<MemberAddress> and simply passed in MemberId in ViewBag. Sure, we wouldn’t have got compile time checking and our controller (and view) would build even if the ViewBag.MemberId were not passed in to the controller. The controller side of things could be rectified with a single unit test [ ((long)(ViewBag.MemberId)).ShouldEqual(5) ] – you achieve the same “notification” of the controller not working, you just get it one step later (at unit test time and not at build time). This would be a concern only if you accept code that fails unit tests as “complete”. I don’t, hence it’s not a problem for me. Loss of testability? I don’t think so.
As for the view side of things – yes, you won’t be able to do Model.MemberId, rather you’ll be doing ViewBag.MemberId. Other than that, the difference is quite minimal. You’d still UI test the page to ensure that the correct variable is getting set properly and if not, it’ll get caught.
Benefits of using ViewBag for some data
Apart from the example mentioned above, ViewBag is quite useful when passing bits of data to the view that aren’t related to the immediate functionality of the view. For example, widget data or navigation data can be set up in a base controller and put in the ViewBag. If you were to use strongly typed viewmodels, you would have to adhere to a hierarchy of viewmodel classes which would add complexity and rigidity. Anything that doesn’t quite fit into your existing vm hierarchy will be an annoyance and may require time and effort in maintaining your one true viewmodel tree. All that work to pass in some trivial data not directly related to the immediate functionality of the view – seems like a waste. If you set the secondary bits of data via the ViewBag, this becomes so much simpler. And I’m all for simplification. This does mean you’ll need to check whether or not a controller is setting the appropriate ViewBag entry via a unit test. But one test vs all that complexity – it’s really a no brainer for me.
So you’re saying not to use strongly typed viewmodels?
No I’m not. I’m all for strongly typed viewmodel for the data that is of immediate concern to the view’s functionality. For example, on a EditMember page, the MemberDetails might be the viewmodel. However, NavigationItems, WidgetData, CountryDropDownListItems etc. do not contribute directly to the MemberDetails editing feature. As such, I would put those bits in viewbag while keeping the view’s type as MemberDetails (as opposed to a MemberDetailsWithNavigationAndWidgetAndCountryItemsViewModel). You can see a bit of this in the default project template as well – they set the page’s title in ViewBag and display it in the master from ViewBag.