Wednesday, December 18, 2013

Say No to ViewData and ViewBag

We've all been there, we got a perfect Model/ViewModel for the view…


public class HomeController : Controller
{

    public ActionResult Index()
    {

        var people = new[]
        {
            new Person { Firstname = "Ely", Lastname = "Buendia"},
            new Person { Firstname = "Raymund", Lastname = "Marasigan"},
            new Person { Firstname = "Buddy", Lastname = "Zabala"},
            new Person { Firstname = "Marcus", Lastname = "Adoro"},
        };

        return View(people);

    } // Index Action

} //Home Controller    


View:

@model IEnumerable<TestAdhocViewModel.Models.Person>

 
<table>    
@foreach (var p in Model)
{
    <tr>
        <td>@p.Firstname</td>
        <td>@p.Lastname</td>
    </tr>
}
</table>


...and then one day, there's a new requirement to bring new info on the view.

You can either put it in ViewData or ViewBag. However, you reluctantly wanted to put those info on ViewData/ViewBag. It pains you to see the contract(Model/ViewModel) between the controller and the view is being violated in the broad daylight, the various information are located in disparate sources, some of them are stored in strongly-typed Model/ViewModel but some are in ViewData/ViewBag. ViewData/ViewBag mars the beauty of operating exclusively within the domain of Model/ViewModel.


You can serialize/deserialize the anonymous types with JSON, but you will lose the benefit of autocomplete. Dynamic types are not navigable. Just because you could doesn't mean you should


So it's safe to say we all love our strongly-typed language. C#, a language so strongly-typed so we don't have to type strongly, thanks autocomplete! :D


We can introduce a view model, but it's too ceremonial when we just wanted to bring only a few new info to view. Scratch that, there's nothing ceremonial with view model when it's done on an ad hoc approach. Whether or not there is really a place for adhocly things to occur on a very formalized system(system that leads to class explosions(read: gazillion of classes), tends to be rampant on strongly-typed language) is up for another contentious discussion; we will not go into that.


When an approach is too ceremonial(read: tedious), we tend to forego that approach, we tend to use the duct tape approach, e.g., we tend to use ViewData/ViewBag, Session


What we really don't want is a view model that is just being used on one part of the system only, yet it pollutes the global namespace; classes in global namespace tends to give the illusion that a view model is very reusable. We wanted that ad hoc view model to be local on one part of the system only, that is that view model should be visible on that part only.


Without further ado, just declare the ViewModel right on the controller only

public class HomeController : Controller
{

    public ActionResult Index()
    {

        var people = new[]
            {
                new Person { Firstname = "Ely", Lastname = "Buendia"},
                new Person { Firstname = "Raymund", Lastname = "Marasigan"},
                new Person { Firstname = "Buddy", Lastname = "Zabala"},
                new Person { Firstname = "Marcus", Lastname = "Adoro"},
            };


        var band = new Band
            {
                BandName = "Eraserheads",
                Members = people
            };
        
        return View(band);

    } // Index Action


    public class Band
    {
        public string BandName { get; set; }
        public IEnumerable<Person> Members { get; set; }
    }

} //Home Controller



View:


@model TestAdhocViewModel.Controllers.HomeController.Band

 
@Model.BandName

 
<table>        
    @foreach (var p in Model.Members)
    {
        <tr>
            <td>@p.Firstname</td>
            <td>@p.Lastname</td>
        </tr>
    }
</table>


If it still feels like you violated a contract between your controller and view, as there's a big impact on your code when the shape of your object changes, i.e., the code changed to: foreach(var p in Model.Members) instead of retaining the old code: foreach(var p in Model), and you wanted a lesser change in your code, tap the features of your language. In C#, you can provide an enumerator for your class instance. Redefine your class to this:


public class Band : IEnumerable<Person>
{
    public string BandName { get; set; }
    // using internal, Members property is accessible to controller only, is not visible on views
    internal IEnumerable<Person> Members { get; set; }

    public IEnumerator<Person> GetEnumerator() { return Members.GetEnumerator(); }
    IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }
}

With that, you can use the old code on foreach, i.e., you can just use the Model directly on foreach:


@model TestAdhocViewModel.Controllers.HomeController.Band

 
@Model.BandName

 
<table>        
    @foreach (var p in Model)
    {
        <tr>
            <td>@p.Firstname</td>
            <td>@p.Lastname</td>
        </tr>
    }
</table>


For single object, you can apply the same concept above, just use inheritance, if you have this code..

public ViewResult Detail(int id)
{
    var p = new Person
        {
            Firstname = "Ely",
            Lastname = "Buendia"
        };
    
    return View(qp);
} 

View:
@model TestAdhocViewModel.Models.Person

 
<p>Firstname: @Model.Firstname</p>
<p>Lastname: @Model.Lastname</p>


..and you wanted just to bring another information to view, create a derived class inside the controller and inherit the existing Model/ViewModel:

public class HomeController : Controller
{
    public ViewResult Detail(int id)
    {
        var p = new Person
            {
                Firstname = "Ely",
                Lastname = "Buendia"
            };
        var qp = new QualifiedPerson();
        qp.Firstname = p.Firstname;
        qp.Lastname = p.Lastname;
        qp.IsQualified = true;
        
        return View(qp);
    } 

    public class QualifiedPerson : Person
    {
        public bool IsQualified { get; set; }
    }
    
} //Home Controller

This is now your view with minimal modification, just change the model on model directive:


@model TestAdhocViewModel.Controllers.HomeController.QualifiedPerson
 
<p>Firstname: @Model.Firstname</p>  @* Look Ma! No Changes! *@
<p>Lastname: @Model.Lastname</p> @* Look Pa! No Changes! *@
<p>Qualified: @Model.IsQualified</p>

Assigning each values from base class to derived class is tedious though. Just use the awesome ValueInjecter, a.k.a. Omu Value Injecter



public class HomeController : Controller
{
    public ViewResult Detail(int id)
    {
        var p = new Person
            {
                Firstname = "Ely",
                Lastname = "Buendia"
            };
        var qp = new QualifiedPerson();
        qp.InjectFrom(p);
 
        qp.IsQualified = true;

        return View(qp);
    } 

    public class QualifiedPerson : Person
    {
        public bool IsQualified { get; set; }
    }
    
} //Home Controller


Then just use the InjectFrom extension method to copy the values from base class to derived class


Another beauty of placing the new information to their adhoc model/viewmodel, when the model/viewmodel doesn't feel adhocly anymore (e.g., it can be reused, merging multiple model/viewmodel service calls into one), it's easy to move that model/viewmodel to global namespace (e.g., to WCF), and it will only incur minimal adjustments to your controller and view. Contrast that to passing your related information to view in a non-cohesive manner, i.e., some of the info are in model/viewmodel and some are in ViewData/ViewBag, it will incur more adjustments to your code to use the refactored related info. Use ViewData and ViewBag sparingly.


Can we say goodbye to ViewData and ViewBag now?


Happy Coding! ツ

No comments:

Post a Comment