Mapping List to a model in ASP.NET MVC5

Question

I am trying to map my model with the view model and I guess this isn't the most efficient way. Here is the code:

List<hall> allHalls = db.halls.Take(30).ToList();
List<HallViewModel> HVMLIST = new List<HallViewModel>();

int process = 0;

foreach(var hall in allHalls)
{
    havvViewModel HVM = new havvViewModel();
    HVM.name = hall.name;
    ...

}

Is there a more efficient way to do this? Will calling havvViewModel HVM = new havvViewModel(); in a for loop create a performance issue since I'm making a new object every time? Please Advise...

Take a look at [AutoMapper](https://github.com/AutoMapper/AutoMapper). — , Sep 02 '14 at 05:10
Making a new instance of havvViewModel will NOT create performance issues because it's really, really, really fast. — labilbe, Sep 02 '14 at 07:40

score 1 · Accepted Answer · answered Sep 02 '14 at 12:13

The way your code is written, there really isn't anything wrong with it from a performance standpoint. Creating a new object is a relatively inexpensive operation (assuming there isn't any work being done in the constructor) and creating 30 objects is nothing to be concerned about.

If you wanted to you could make your code linq-y. This won't really have a performance impact, but it looks cool :)

return 
    db.halls
      .Take(30)
      .Select(h => 
          new havvViewModel
          {
              Name = h.name
          });

score 0 · Answer 2 · answered Sep 02 '14 at 12:14

0

As @labilbe commented, unless you are constructing thousands and thousands of objects in that loop, its going to execute instantly. My preference is to have one ViewModel per "screen" (roughly) and if I had a page that rendered a list of halls, I'd compose the ViewModel like

public class HallListing : BaseViewModel
{
     private List<hall> halls;
     public void LoadData() 
     {
          this.halls = base.db.halls.Take(30).ToList();
     }
}

abstract class BaseViewModel 
{
     protected DataContext db { get; private set; }
     public BaseViewModel() 
     {
          this.db = new DataContext();
     }
}

answered Sep 02 '14 at 12:14

Graham

3,217
1
27
29

Models shouldn't be loading themselves from the database. That's the Controller's job. – Philip Pittle Sep 02 '14 at 12:29
I've often heard that approach stated, but could never see the benefit of it personally. I just want my Controller to map a route to a ViewModel, and execute a bare mimimum of public methods on those ViewModels. – Graham Sep 02 '14 at 12:32
We should probably move our discussion on the topic here: http://stackoverflow.com/questions/13951445/yes-or-no-should-models-in-mvc-contain-application-logic#13952171. For the sake of this question, it's not particularly relevant. – Philip Pittle Sep 02 '14 at 13:23

Mapping List to a model in ASP.NET MVC5

2 Answers2