Skip to main content
Code Review

Return to Question

Added 4 paragraphs at the bottom, detailing an 'ideal' situation and some more questions
Source Link
marin
marin
  • 163
  • 1
  • 6

Ideally, the code in my controller method should be:

public ActionResult GetRolesByUserGridData(MvcJqGrid.GridSettings gs, int userId)
{
   var json = _userRoleService
      .GetRolesByUser()
      .ToOrderedList(“Id”, “Asc”)
      .ToPagedList(gs.PageIndex, gs.PageSize)
      .ToJsonData(); 
   return Json(jsonData, JsonRequestBehavior.AllowGet);
}

where ToOrderedList(), ToPagedList() and ToJsonData() would be extension methods to IQueryable<>, placed outside the Web project.

But it's still not clear to me where should the ViewModels live in this scenario:

A. Have them in the Service layer, which is in a different project - but then they lose all the logic related to Data Annotation, or I need to reference MVC in the Service layer, which seems weird.

B. Have them in the Web project - but why shouldn't the service be aware of the ViewModel and only retrieve the data that the ViewModel needs? Is it really necessary to create a new DTO object only to pass data from the service to the controller?

Ideally, the code in my controller method should be:

public ActionResult GetRolesByUserGridData(MvcJqGrid.GridSettings gs, int userId)
{
   var json = _userRoleService
      .GetRolesByUser()
      .ToOrderedList(“Id”, “Asc”)
      .ToPagedList(gs.PageIndex, gs.PageSize)
      .ToJsonData(); 
   return Json(jsonData, JsonRequestBehavior.AllowGet);
}

where ToOrderedList(), ToPagedList() and ToJsonData() would be extension methods to IQueryable<>, placed outside the Web project.

But it's still not clear to me where should the ViewModels live in this scenario:

A. Have them in the Service layer, which is in a different project - but then they lose all the logic related to Data Annotation, or I need to reference MVC in the Service layer, which seems weird.

B. Have them in the Web project - but why shouldn't the service be aware of the ViewModel and only retrieve the data that the ViewModel needs? Is it really necessary to create a new DTO object only to pass data from the service to the controller?

added 2 characters in body; edited tags
Source Link
Jamal
Jamal
  • 35.2k
  • 13
  • 134
  • 238

My goal is to return IEnumerableIEnumerable from repositories and have fairly dumb controllers.

My goal is to return IEnumerable from repositories and have fairly dumb controllers.

My goal is to return IEnumerable from repositories and have fairly dumb controllers.

Source Link
marin
marin
  • 163
  • 1
  • 6

Refactoring fat ASP.NET MVC Controller

I have just began porting an old project to ASP.NET MVC . In the end, I'll have a lot of controller methods like the one below, called by AJAX requests done by JQGrid objects in the pages:

AJAX request:

/UserRole/RolesByUserGridData?userId=2&_search=false&nd=1407171194811&rows=10&page=1&sidx=RoleName&sord=asc

Controller code:

public ActionResult GetRolesByUserGridData(MvcJqGrid.GridSettings gridSettings, int userId)
    {
        IQueryable<Role> allRoles = _roleRepository.GetAsQueryable(where:null, includeProperties:"");
        IQueryable<UserRole> userRolesByUser = _repository.GetAsQueryable(where: c => c.UserId == userId, includeProperties: "Role");

        var gridItems = from role in allRoles
                        join userRole in userRolesByUser on role.Id equals userRole.RoleId into loj
                        from item in loj.DefaultIfEmpty()   // LEFT OUTER JOIN equivalent 
                        select new UserRoleByUserViewModel
                        {
                            IsRoleAssociatedWithUser = (item.UserId == userId),
                            RoleName = role.Name
                        };
        
        SortOrder sortOrder = gridSettings.SortOrder != "desc" ? SortOrder.Asc : SortOrder.Desc;

        var pagedOrderedItems = PagingHelper<UserRoleByUserViewModel>.GetAsOrderedPagedList(
            gridItems, sortOrder, gridSettings.SortColumn,
            gridSettings.PageIndex, gridSettings.PageSize);
        
        var jsonData = new
        {
            total = pagedOrderedItems.TotalItemCount / gridSettings.PageSize + 1,
            page = gridSettings.PageIndex,
            records = pagedOrderedItems.TotalItemCount,
            rows = (
                from c in pagedOrderedItems
                select new
                {
                    id = "",
                    cell = new[]
                    {
                        "Edit", 
                        "Details",
                        c.IsRoleAssociatedWithUser.ToString(),
                        c.RoleName
                    }
                }).ToArray()
        };

        return Json(jsonData, JsonRequestBehavior.AllowGet);
    }

My goal is to return IEnumerable from repositories and have fairly dumb controllers.

How would you suggest refactoring this?