Showing related data in viewmodel, a better way?

Question

My aim is to display data from two tables in my HTML page in one table/grid.

ID, Forename, Surname, Hobby Name

Tables

People(ID, Forename, Surname, Hobby ID)

Hobby(ID, Hobby Name)

namespace DC.ViewModels
{

    public class PeopleHobbyView()
    {
        public int ID {get; set;}
        public string Forename {get; set;}
        public string Surname {get; set;}
        public string Hobby_Name {get; set;}
    }

    public class PeopleHobbiesVM
    {
        public List<PeopleHobbyView> PeopleList { get; set; }

        public void GetPeople()
        {
            using (var context = new LSF1617Entities())
            {
                var people = (from people in context.People join hobbies in context.Hobby on people.Hobby_ID equals hobbies.ID
                    select new PeopleHobbyView { ID = people.ID, Forename = people.Forename, Surname = people.Surname, Hobby_Name = hobbies.Hobby_Name}
                );
                this.PeopleList = people.ToList();
            }
        }
    }
}

Hobby name has to be of course looked up in the Hobby table, so I have created a join in my GetPeople() method

This all seems a bit long winded, I just wanted to get reassurance that this is a good way of going about the task. I have read a bit about lazy/eager loading, and I can't seem to get my head around it completely.

Turn off lazy loading, use Include to explicitly include required navigation properties. — L-Four, Jul 01 '16 at 13:34
Quick questions: PeopleList is a list of People and people is a list of PeoplHobbyView, how does this work? Does People inherit from PeopleHobbyHiew — Andrew Kilburn, Jul 01 '16 at 13:34

Fran · Answer 1 · 2016-07-01T13:33:26.767

1

This is a perfectly fine way to query data using EF. you could use navigation properties if you like

It would remove this line

join hobbies in context.Hobby on people.Hobby_ID equals hobbies.ID

as it would be declared in the navigation property.

As for the lazy loading, you really aren't going to use it in this scenario. You might want to page your results if they are going to be very long.

I would move this code out of the view model and into a controller action. you usually are going to have your context passed in to the controller's constructor.

public class SomeController : Controller {
    public SomeController(LSF1617Entities dbContext)
    {
        _dbContext = dbContext;
    }

    public ActionResult Index()
    {
        var model = (from people in context.People join hobbies in context.Hobby on people.Hobby_ID equals hobbies.ID
                    select new PeopleHobbyView { ID = people.ID, Forename = people.Forename, Surname = people.Surname, Hobby_Name = hobbies.Hobby_Name}
                ).ToList();

        return View(model);
    }

edited Jul 01 '16 at 13:33

answered Jul 01 '16 at 13:28

Fran

6,440
1
23
35

I thought your database logic was supposed to stay out of the controller? – Danny Cullen Jul 01 '16 at 13:37
It depends what pattern you use, i personally store my db code in a repository so it goes View --> controller --> repo – Ben Jones Jul 01 '16 at 13:38
Actually, the dbContext and the Linq should be in a service layer, not the controller layer. Sure, transform the model from the dto (the linq line) to the view model in the controller, but don't go querying data in controllers, that's a bad code smell (controllers are notoriously difficult to unit test). – Neil Jul 01 '16 at 13:38
1

I agree, but I didn't want to overload Joakhim with repositories and service layers. Most of my code looks like what @Ben Jones describes. – Fran Jul 01 '16 at 13:42
@Neil. Controllers aren't difficult to unit. in the scenario Ben Jones described. They might be going directly against a DbContext. – Fran Jul 01 '16 at 13:42
@Fran Controllers ARE difficult to test. How do you mock (or Moq) all the HttpContext stuff? nightmare! – Neil Jul 01 '16 at 13:47
public static void SetControllerContext(this Controller controller) { var fakeContext = A.Fake(); var fakeRequest = A.Fake(); var fakeResponse = A.Fake(); A.CallTo(() => fakeRequest.HttpMethod).Returns(HttpVerbs.Post.ToString()); A.CallTo(() => fakeContext.Response).Returns(fakeResponse); A.CallTo(() => fakeContext.Request).Returns(fakeRequest); var fakeRequestContext = new RequestContext(fakeContext, new RouteData()); – Fran Jul 01 '16 at 13:52
controller.ControllerContext = new ControllerContext(fakeRequestContext, controller); } – Fran Jul 01 '16 at 13:52

Joakim Hansson · Answer 2 · 2016-07-01T13:41:31.210

1

You are almost correct.

However, instead of doing it like this I would create two seperate models. One for 'People' and one for 'Hobbies'. This makes it easier to read.

public class People
{
   public int Id { get; set; }
   public string Forename { get; set; }
   public string Surname { get; set; }

   public Hobby Hobby { get; set; }
}

public class Hobby
{
   public int Id { get; set; }
   public string Name { get; set; }

   public ICollection<People> People { get; set; }
}

In your context you can then load the relationship. The method for this depends a little bit on if you want to do lazy loading or not. Please read this article:

https://msdn.microsoft.com/en-us/data/jj574232.aspx

As long as the relationship exists in your database and the loading is setup, whenever you access a People object it will also show you the related hobby.

edited Jul 01 '16 at 13:41

answered Jul 01 '16 at 13:35

Joakim Hansson

544
3
15

1

ViewModel can be a flat model, as long as it's the most optimized way for the view. So the initial view model is perfectly fine and should not necessarily be seperated. – L-Four Jul 01 '16 at 13:38
@L-Four Yes, the viewmodel is fine. However, it would make more sense to put the people properties from the view into a seperate model. – Joakim Hansson Jul 01 '16 at 13:39
Why? It's called a flat view model, flat as opposed to composition. See http://stackoverflow.com/questions/6954102/asp-net-mvc-architecture-viewmodel-by-composition-inheritance-or-duplication. It represents a model for the view, so flattened is the most logical choice. – L-Four Jul 01 '16 at 13:42
@L-Four I edited my answer as it was unclear that it was a suggestion to make it easier for him to understand lazy/eager loading. – Joakim Hansson Jul 01 '16 at 13:44
1

I would avoid lazy loading as it can give you unexpected performance issues. If you know upfront that the property is needed, then load it explicitly using the Include statetement. – L-Four Jul 01 '16 at 13:48
I have been using my ViewModels to contain data i'm displaying on my page. I also use them for defining methods such as Model.Create() Model.Delete(int ID) which talk to my database. Should I put those elsewhere? – Danny Cullen Jul 01 '16 at 14:06
1

Using the viewmodel to contain data that you display for your page is perfectly fine. I would however move any logic to your controllers. If you want, you could also create a service layer that sits between your presentation layer and data access layer. Your presentation layer would then instead call methods from the service layer which in turn talks to the data access layer :) – Joakim Hansson Jul 01 '16 at 14:08

Ben Jones · Accepted Answer · 2016-07-01T13:46:48.857

0

Try this

PeopleList = context.People.Include(x => x.Hobby).select(x => new PeopleHobbyView {
      D = x.ID,
      Forename = x.Forename,
      Surname = x.Surname,
      Hobby_Name = x.Hobby.Hobby_Name
}).toList();

there isn't much difference and your way is perfectly fine though, only difference is the toList on the end of the query and assigning into PeopleList without the var people

edited Jul 01 '16 at 13:46

answered Jul 01 '16 at 13:30

Ben Jones

699
4
18

in this example, how does it know about `people` and `hobbies` when assigning? – Danny Cullen Jul 01 '16 at 13:45
the include with bring back the hobby associated with the people, and before you were doing this.PeopleList = people.ToList(); and all i did was add tolist to the query iself and assign it directly – Ben Jones Jul 01 '16 at 13:48
I need to turn off lazy loading before I can use the .Include I think. – Danny Cullen Jul 01 '16 at 14:33
Not that i am aware? :) – Ben Jones Jul 01 '16 at 14:39
I was missing using System.Data.Entity; – Danny Cullen Jul 01 '16 at 14:46

Showing related data in viewmodel, a better way?

3 Answers3