current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I managed to put together following code to correctly Populate RequestExecutionViewModel model,however this looks extremely verbose to me. I feel some sort of LINQ builtin mapping construct can be used to reduce the over all code size. Please suggest improvements.

string userName = User.Identity.Name;
List<Requests> requests = db.Requests.Where(a => a.UserName.
                          Equals(userName)).ToList();
List<RequestExecutionViewModel> items = new List<RequestExecutionViewModel>();
foreach (Requests request in requests) 
{ 
  RequestExecutionViewModel requestview = new RequestExecutionViewModel();
  requestview.Request = request;
  requestview.ExecutionStatus = db.ExecutionStatus.Where(a =>    a.RequestId.Equals(request.RequestsId)).ToList();
  requestview.Approvals = db.Approvals.Where(a =>  a.RulesId.Equals(request.RulesId)).ToList();
  items.Add(requestview);
}

Edit: With the help of jessehouwing answer I restructured my models and removed the need for RequestExecutionViewModel entirely. Now my query is simplified to this.

var requests = db.Requests.Include(r => r.Rule);
share|improve this question

migrated from stackoverflow.com Feb 22 '12 at 5:15

This question came from our site for professional and enthusiast programmers.

2 Answers 2

up vote 1 down vote accepted

It's hard to tell exactly how to write this query without knowing exactly what your datamodel look like and how you've set it up. An assumption was made you're using Linq 2 SQL, I'll adhere to that assumption unless we hear otherwise. You can eagerly fetch other tables if you tell linq to sql to do so. You use the Include method for that.

If you've set up your datamodel correctly, you should be able to create a query that resembles this:

string userName = User.Identity.Name;
List<RequestExecutionViewModel> = (from request in db.Requests.Include("Requests.ExecutionStatus").Include("Requests.Rules.Approvals");
    where  request.UserName == userName
    select new RequestExecutionViewModel(){
        Request = request,
        ExecutionStatus = request.ExecutionStatus,
        Approvals = request.Rule.Approvals
    }).ToList();

And as you can see, this query instructs Linq-to-SQL to eagerly fetch the to related tables and then uses them in the output shaping process to create your viewmodel directly..

Make sure you look at the profiler to see that this is actually resulting in one query. Linq-to-SQL can at any time decide to loop over the subresults by itself.

share|improve this answer
    
thanks this is the construct I was looking for. –  drieddust Feb 23 '12 at 1:59
up vote 0 down vote

What is going to give you massive overhead is pulling back requests then looping through all of the requests and finding associated table data. In reality, you should be building a single query (stored procedure) that will pull back all requests and will also return tables for ExecutionStatus and Approvals that you can easily filter with LINQ to isolate the rows you want.

Going to the database to fetch ALL data once will greatly speed up your application.

share|improve this answer
    
I totally understand the performance penalty. I am looking for help to improve it. –  drieddust Feb 22 '12 at 3:35
    
Are you using LINQ to SQL? I assume you are. Are you familiar with stored procedures? Here is an older article that described how to return more than one result set using LINQ to SQL: blogs.msdn.com/b/swiss_dpe_team/archive/2008/02/04/… Your goal should be to create a single stored procedure that can perform all of the data extraction in a single execution and then do what ever filtering you need AFTER the data is in your application using LINQ. –  Splash-X Feb 22 '12 at 3:56
    
thanks but I do not want to use stored procedures.Moreover RequestExecutionViewModel do not actually exist in database so I am not sure how SP is going to work on it.There should be a method to do it right inside the controller using LINQ. Being new to ASP.NET and LINQ I am having hard time figuring it out. –  drieddust Feb 22 '12 at 4:29

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.