Is there a more efficient way to query data and pass as a View Model List to a View?

Question

I have an Item model mapping to the DB like so:

public class Item
{
    public int ItemId { get; set; }
    public DateTime Created { get; set; }
    public string Title { get; set; }
}

To display lists of these items, I have created a ItemSummaryViewModel like so:

public class ItemSummaryViewModel 
{
    public int ItemId { get; set; }
    public string Title { get; set; }   

    public ItemSummaryViewModel(Item item)
    {
        this.ItemId = item.ItemId;
        this.Title = item.JobTitle + " " + item.Created.ToString("ddd d MMM HH:mm");
    }
}

I have also created a class to take a List< Item > and return a List< ItemSummaryViewModels > like so:

public class ItemSummaryViewModelList : List<ItemSummaryViewModel>
{
    public ItemSummaryViewModelList(List<Item> items)
    {
        foreach (Item i in items)
        {
            ItemSummaryViewModel itemSummary = new ItemSummaryViewModel(i);
            this.Add(itemSummary);
        }
    }
}

Finally, we use the controller to pass the list into the View like so:

    public ActionResult Index()
    {
        //IEnumerable<ItemSummaryViewModel> itemsummaries = new IEnumerable<ItemSummaryViewModel>();

        List<Item> ListOfItems = db.Items.ToList();

        ItemSummaryViewModelList ListOfItemViewModels = new ItemSummaryViewModelList(ListOfItems);

        return View(ListOfItemViewModels);
    }

My Questions Are:

1. Is there a more efficient or "best practice" way of doing this?

2. To transform the list of DB models (Item) into a list of displayable View Models (ItemSummaryViewModels), we currently iterate through each item in the list and transform them individually. Is there a more efficient way of doing this ?

Essentially we're querying the DB and assigning the data to a ViewModel for display as a list. I can't help feeling that I'm "going round the houses" a bit and that there might be a more efficient or "best practice way of doing this.

Is there a better way?

Thanks

Answer 1

Try using LINQ select:

List<ItemSummaryViewModel> results = items.Select(
            x =>
            new ItemSummaryViewModel
                {
                    ItemId = x.ItemId,
                    Title = x.Title + " " + x.Created.ToString("ddd d MMM HH:mm")
                }).ToList();

Put that list in your view model.

Answer 2

Regarding efficiency, I would not worry until you have found that the simplest to implement solution was overly slow in practice. Get it working first and then only optimise when actually necessary. Obviously in the example you give, there are opportunities to only query and convert the subset of Items that the view requires (may it is all, but maybe you are paging?)

Structurally, I think the academic and professionally correct answer would be to have one set of objects to represent your database entities, a second set to represent the "domain" or business objects, and a third set to represent the all of the MVC models. However, depending the exact scenario this could be simplified:

1. If there is a really close mapping between the business objects and the database entities, and it is very unlikely that the database is going to change significantly, then you could have a single class for both.

2. If you have a very simple set of views that map very cleanly onto your business objects, then perhaps you can use business objects as your models. Unless your views do nothing but splat raw business objects onto a web page, I think your models will normally need to be more complicated than your current example though.

For that specific case, I would agree with @CorrugatedAir and say you could just use a plain List rather than create your own List class, and if want to be simpler, you could just use List and skip creating the ItemSummaryViewModel class too.

But try and be consistent throughout the application - so if you find a situation where your database entities can't be used as business objects, then it is best to have a separate set in all instances and have mappers between them.

Answer 3

To answer the "best practice" part of your question:

More efficient way (architecturally) will be to use Unit of Work and the repository patterns. That way you decouple your views from your data source, making it more reusable, more testable, more readable, hence more maintainable along with other "more"s.

The article is very graphical and gives you real feel of why do you need to tear apart database access from the controller.

To answer the technical part of how to transform it in a less verbose way,

I'd use something called AutoMapper. Using it, your complex transformation instead of the loop you presented will look as something like this:

public ActionResult Index()
{
  var dbList = db.Items.ToList();
  var vmList = Mapper.Map<List<Item>, List<ItemSummaryViewModel>>(dbList);
  return View(vmList);
}

You will also have to put this initialization somewhere in your App_Start configuration (if MVC 4) or in Global.asax.cs file:

Mapper.CreateMap<ListOfItems , ItemSummaryViewModelList>();
Mapper.AssertConfigurationIsValid();

You can read more about why use AutoMapper and how to use it AutoMapper: Getting Started

Hope this helps!