MVC ViewModels and Entity Framework queries

Question

I am new to both MVC and Entity Framework and I have a question about the right/preferred way to do this.

I have sort of been following the Nerd Dinner MVC application for how I am writing this application. I have a page that has data from a few different places. It shows details that come from a few different tables and also has a dropdown list from a lookup table.

I created a ViewModel class that contains all of this information:

class DetailsViewModel {
    public List<Foo> DropdownListData { get; set; }

    // comes from table 1
    public string Property1 { get; set; } 
    public string Property2 { get; set; }

    public Bar SomeBarObject { get; set; } // comes from table 2
}

In the Nerd Dinner code, their examples is a little too simplistic. The DinnerFormViewModel takes in a single entity: Dinner. Based on the Dinner it creates a SelectList for the countries based on the dinners location.

Because of the simplicity, their data access code is also pretty simple. He has a simple DinnerRepository with a method called GetDinner(). In his action methods he can do simple things like:

Dinner dinner = new Dinner();

// return the view model
return View(new DinnerFormViewModel(dinner));

OR

Dinner dinner = repository.GetDinner(id);

return View(new DinnerFormViewModel(dinner));

My query is a lot more complex than this, pulling from multiple tables...creating an anonymous type:

var query = from a in ctx.Table1
            where a.Id == id
            select new { a.Property1, a.Property2, a.Foo, a.Bar };

My question is as follows:

What should my repository class look like? Should the repository class return the ViewModel itself? That doesn't seem like the right way to do things, since the ViewModel sort of implies it is being used in a view. Since my query is returning an anonymous object, how do I return that from my repository so I can construct the ViewModel in my controller actions?

Shigella

+1 for question, was wondering this myself. Have only toyed with MVC; haven't had a chance to use it in any real enterprise capacity yet. And unfortunately, you can't return anonymous types from methods. (#2401008) – Mother

Answer 1

You are correct a repository should not return a view model. As changes to your view will cause you to change your data layer.

Your repository should be an aggregate root. If your property1, property2, Foo, Bar are related in some way I would extract a new class to handle this.

public class FooBarDetails
{
   public string Property1 {get;set;}
   public string Property2 {get;set;}
   public Foo Foo {get;set;}
   public Bar Bar {get;set;}
}

var details = _repo.GetDetails(detailId);

If Foo and Bar are not related at all it might be an option to introduce a service to compose your FooBarDetails.

FooBarDetails details = _service.GetFooBar(id);

where GetFooBar(int) would look something like this:

_fooRepo.Get(id);
_barRepo.Get(id);

return new FooBarDetails{Foo = foo, Bar = bar, Property1 = "something", Property2 = "something else"};

This all is conjecture since the design of the repository really depends on your domain. Using generic terms makes it hard to develop potential relationships between your objects.

Updated From the comment if we are dealing with an aggregate root of an Order. An order would have the OrderItem and also the customer that placed the order.

public class Order
{
    public List<OrderItem> Items{get; private set;}
    public Customer OrderedBy {get; private set;}
    //Other stuff
}

public class Customer
{
  public List<Orders> Orders{get;set;}
}

Your repo should return a fully hydrated order object.

var order = _rep.Get(orderId);

Since your order has all the information needed I would pass the order directly to the view model.

public class OrderDetailsViewModel
{
  public Order Order {get;set;}
  public OrderDetailsViewModel(Order order)
  {
    Order = order;
  }
}

Now having a viewmodel with only one item might seem overkill (and it most likely will be at first). If you need to display more items on your view it starts to help.

public class OrderDetailsViewModel
{
  public Order Order {get;set;}
  public List<Order> SimilarOrders {get;set;}
  public OrderDetailsViewModel(Order order, List<Order> similarOrders)
  {
    Order = order;
    SimilarOrders = similarOrders;
  }
}

Lukash

Ok I read the link about the aggregate root. These properties are related to each other. Most of them are navigation properties of my entities. Using a simple, non-generic example. Lets say my view is looking at an Order, and displaying all Order Items / Customer info for the order. The Aggregate Root is the Order. I create an OrderViewModel that passes in an Order object to the constructor. Does the OrderViewModel also have a List<OrderItem> and Customer property? Or does it not need them since they are directly retrieved from the Order itself? – Shigella

@MarkColeman: Doesn't this method cause decrease the performance? – Phage

Answer 2

While most of the answers are good, I think they are missing an in-between lines part of your question.

First of all, there is no 100% right way to go about it, and I wouldn't get too hung up on the details of the exact pattern to use yet. As your application gets more and more developped you will start seeing what's working and what's not, and figure out how to best change it to work for you and your application. I just got done completely changing the pattern of my Asp.Net MVC backend, mostly because a lot of advice I found wasn't working for what I was trying to do.

That being said, look at your layers by what they are supposed to do. The repository layer is solely meant for adding/removing/and editing data from your data source. It doesn't know how that data is going to be used, and frankly it doesn't care. Therefore, repositories should just return your EF entities.

The part of your question that other seem to be missing is that you need an additional layer in between your controllers and the repositories, usually called the service layer or business layer. This layer contains various classes (however you want to organize them) that get called by controllers. Each of these classes will call the repository to retrieve the desired data, and then convert them into the view models that your controllers will end up using.

This service/business layer is where your business logic goes (and if you think about it, converting an entity into a view model is business logic, as it's defining how your application is actually going to use that data). This means that you don't have to call specific conversion methods or anything. The idea is you tell your service/business layer what you want to do, and it gives you business entities (view models) back, with your controllers having no knowledge of the actual database structure or how the data was retrieved.

The service layer should be the only layer that calls repository classes as well.

Heteroousian

Thanks for the explanation. Do you have any example code for a service/business layer that I could look at? I understand what you are saying but I would like an example so I can see some "real world" applications. If there is something in CodePlex or even just a simple Nerd Dinner style example on a blog somewhere that would be helpful. – Shigella

Try looking at asp.net/mvc/tutorials/validating-with-a-service-layer-cs. You would use the same type of thing as the ProductService, but instead of just returning _repository.ListProducts() you would convert ListProducts() into a view model and return that. – Heteroousian

I think this is actually bad advice, because it implies your service layer should depend on your presentation layer (by taking a dependency on the view model classes). The dependency graph should be presentation -> service -> data, not presentation <-> service -> data – Salomie

Maybe, but I don't think that dependency graph is 100% must follow. I tried to follow that but stopped because how I'm presenting the data is actually very similar to how my business logic is aggregating and retrieving/storing/editing data anyway, so for me it fit more in the service layer. Furthermore, if you keep separating out presentation from business logic too much you can end up with performance issues, such as too much lazy loading and too many db calls when it's not needed. Like I said originally, no pattern is 100% – Heteroousian

Also one more point. Technically if you follow your dependency graph 100% you would perform the ViewModel conversion in the View not in the controller (because the View is what determines how it needs to present the data) and that is even uglier. If you convert to a ViewModel in the controller you are also tying your controller to how you want to present the data, and that's against the graph. Somewhere along the lines you have to let up and make compromises. – Heteroousian

Um I thought you might have a reasonably pragmatic viewpoint after your first response, but your second makes me think you're not really understanding how multi-tiered architecture works at all... the presentation layer has a dependency on the service layer, and this is expressed by injected the model classes into the view model constructor. The view is not code, and the controller is dumb; the whole point of the view model is to perform that conversion. – Salomie

Maybe I don't, but just my experience is that changes to my view models have usually required changes to my query logic, unless the viewmodels are very similar to the business entities themselves.. If I ever have the exact same query that needs to be transformed into completely separate viewmodels, my approach doesn't stop me from doing what you are saying later on an as-needed basis. My point is, a lot of times there is business logic in creating view models so doing it in the service layer is not that far of a stretch, and more transformations can be done if needed – Heteroousian

After thinking about it, I think we are having 2 separate conversations and talking past each other (SO isn't really formatted for in depth back and forth conversations). I think I'm arguing where you instantiate and return the view model, and you are arguing how you perform the actual conversion. – Heteroousian

Answer 3

Repository should work only with models not anonymous types and it should only implement CRUD operations. If you need some filtering you can add a service layer for that.

For mapping between ViewModels and Models you can use any of mapping libraries, such as Automapper.

Impel

Answer 4

The current answers are very good. I would just point out that you are abusing anonymous types; they should only be used for intermediate transport steps, and never passed to other places in your code (e.g. view model constructors).

My approach would be to inject the view model with all the relevant model classes. E.g. an action method might look like:

var dinner = dinnerRepository.Get(dinnerId);
var bar = barRepository.Get(barId);

var viewModel = new DinnerAndBarFormViewModel(dinner, bar);
return View(viewModel);

Salomie

Answer 5

I have the same doubt of the poster and I am still not convinced. I personally do not like very much the given advice of limiting the repository to just executing basic CRUD operations. IMHO, performances should always be kept into account when developing a real application, and substituting a SQL outer join with two different queries for master-detail relationships doesn't sound too good to me. Also, this way the principle that only needed fields should be queried is completely lost: using this approach, we are forced to always retrieve all the fields of all the tables involved, which is simply crazy in non-toy applications!

Belomancy