ViewModels in Repository

Question

I have read that the repository layer should not deal with ViewModels because of separation of concerns and should instead deal only with Models. This is also true for the service layer (in my case this is where my business logic is). So then the controller is left to deal with the population of the ViewModels.

I have a Model Category:

public class Category
{

 public int ID { get; set; }

 public int? ParentCategoryID { get; set; }


 public virtual ICollection<Product> Products{ get; set; }

 public virtual ICollection<CategoryName> CategoryNames{ get; set; }
}

I have a ViewModel CategoryListViewModel used when displaying all Categories

public class CategoryListViewModel
    {
        public int ID { get; set; }
        public string Name { get; set; }
        public string ParentName { get; set; }
    }

My view takes IEnumerable<...CategoryListViewModel>

This is how I populate the ViewModel from the controller:

public ActionResult Index()
        {            
            IEnumerable<CategoryListViewModel> model;
            List<CategoryListViewModel> list = new List<CategoryListViewModel>();
            IEnumerable<Category> categoryList = categoryService.GetAllCategoriesList(RouteData);

            foreach (var item in categoryList)
            {
                CategoryListViewModel temp = new CategoryListViewModel()
                {
                    ID = item.ID,
                    Name = categoryService.GetCategoryName(RouteData, item.ID)                    
                };
                if (item.ParentCategoryID != null)
                {
                    temp.ParentName = categoryService.GetCategoryName(RouteData, (int)item.ParentCategoryID);
                }
                list.Add(temp);
            }
            model = list;
            return View(model);
        }

My service methods:

public IEnumerable<Category> GetAllCategoriesList(RouteData data)
        {
            LanguageService languageService = new LanguageService();
            Languages langEnum = languageService.LanguageStringToEnum(languageService.DetermineSelectedLanguage(data));

            IEnumerable<Category> allCategories = repository.getAllCategoriesTest();            

            return allCategories;
        }

public string GetCategoryName(RouteData data, int categoryId)
        {
            LanguageService languageService = new LanguageService();
            Languages langEnum = languageService.LanguageStringToEnum(languageService.DetermineSelectedLanguage(data));
            return repository.GetCategoryName(langEnum, categoryId);
        }

And finally my repository methods:

public IEnumerable<Category> getAllCategoriesTest()
        {
            return db.Category.ToList();
        }


public string GetCategoryName(Languages lang, int categoryId)
        {
            return db.CategoryName.Where(cn => cn.CategoryID == categoryId && cn.Language == lang).Select(cn => cn.Name).FirstOrDefault();
        }

This approach looks very bad to me. My Controller is not thin anymore and I am running a lot of queries for something that simple.

If I allow ViewModels in my repository I get a much cleaner solution.

My Controller method:

public ActionResult Index()
        {
            return View(categoryService.GetAllCategories(RouteData));            
        }

Service method:

public IEnumerable<CategoryListViewModel> GetAllCategories(RouteData data)
        {
            LanguageService languageService = new LanguageService();
            Languages langEnum = languageService.LanguageStringToEnum(languageService.DetermineSelectedLanguage(data));

            return repository.SelectAllCategories(langEnum);
        }

And repository method:

public IEnumerable<CategoryListViewModel> SelectAllCategories(Languages lang)
        {                        
            var categories = db.Category.Include(c => c.CategoryNames).Select(names => new CategoryListViewModel
            {
                ID = names.ID,
                Name = names.CategoryNames.Where(cn => cn.Language == lang).Select(cn => cn.Name).FirstOrDefault(),
                ParentName = db.CategoryName.Where(cn => cn.Language == lang && cn.CategoryID == names.ParentCategoryID)
                .Select(cn => cn.Name).FirstOrDefault()
            }).ToList();            
            return categories;                             
        }

This approach, although violating separation of concerns seems to be "cleaner" to me.

My question is isn't the other approach more efficient in term of queries? Also is there any other way that this could be done so as not to write heavy controller methods and not execute that many queries? It seems to me that I am missing something.

Answer 1

First, bear in mind that even though it has "MVC" in the name, ASP.NET MVC only very loosely implements the MVC pattern. MVC tells you to have thin controllers because the Model is an active record, which handles all the business logic, including that around querying itself. This does not apply to ASP.NET MVC. There, your Model is actually a combination of your DAL, service layer, entity and one or more view models. This means the controller inevitably must do at least a little more work than a controller in something like Ruby on Rails, if only to wire all this stuff together.

As @Liam suggested in the comments above, your best bet is factories. That way, the controller does not actually own the logic for how to map an entity to a view model. You'll of course still need to actually call the factory in your controller, but the logic remains abstracted.

Also, a proper service layer should roll up logic that would otherwise be in your controller. If you need the localized name for the category, your service should have a method that returns all the categories with their localized name already. If you're having to hit your service multiple times, that's a clear indication that you haven't provided a necessary endpoint for your application. You may need to introduce a DTO to handle this data, since the entity class may not have the appropriate properties. You'd then have a factory that maps your DTO to a view model.

Finally, for what it's worth, your repository is completely unnecessary. Just have your service interact directly with your Entity Framework context. Having a repository buys you nothing but just an additional thing you have to maintain.