current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

Is there any way to refactor this?

objLeadTimeReportResult = 
    (from prop in propList
     join book in booking on prop.PropertyId equals book.PropertyId into j
     from book in j.DefaultIfEmpty()
     group book by new { prop.PropertyId, prop.PropertyName } into groupedLeadTime
         select new LeadTimeReportResult
         {
             PropertyId = groupedLeadTime.Key.PropertyId,
             PropertyName = groupedLeadTime.Key.PropertyName,
             Today = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days == 0) ? 1 : 0),
             Days1_2 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 1 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 2) ? 1 : 0),
             Days3_6 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 3 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 6) ? 1 : 0),
             Days7_10 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 7 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 10) ? 1 : 0),
             Days11_15 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 11 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 15) ? 1 : 0),
             Days16_30 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 16 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 30) ? 1 : 0),
             Days31_45 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 31 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 45) ? 1 : 0),
             Days46_60 = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days >= 46 && (item.CheckInDate.Date - item.BookingDate.Date).Days <= 60) ? 1 : 0),
             Days60Plus = groupedLeadTime.Sum(item => ((item.CheckInDate.Date - item.BookingDate.Date).Days > 60) ? 1 : 0)
         }).ToList();

Additional Information

My senior programmer told me to use some instead of count, he claimed that it would make better performance when using on MSSQL Server.

share|improve this question
up vote 5 down vote accepted

(1). You can create an exntession method to check that item is within the range.

(2). I'd use Count instead of Sum because you just count the number of days in specified interval. It's more natural.

(3). I'd check performance of this query and try to cache result of this join and/or grouping by using some method that requires eager evaluation. ToList() for example. It can help with a lot of traverals of every group in Sum()/Count() methods.

(4). Note for additional information. Even if Sum() in Linq To SQL is a lot faster than Count() you still can have MyCount() implemented as Sum() inside. This will allow you to have a good performance as your senior programmer mentioned and hide this "? 1 : 0" conidition.

 var tempQuery = from prop in propList
                                       join book in booking on prop.PropertyId equals book.PropertyId into j
                                       from book in j.DefaultIfEmpty()
                                       group book by new { prop.PropertyId, prop.PropertyName }.ToList();

 objLeadTimeReportResult = from tempVar in tempQuery
                                 select new LeadTimeReportResult
                                       {
                                           PropertyId = tempVar.Key.PropertyId,
                                           PropertyName = tempVar.Key.PropertyName,
                                           Today = tempQuery.Count(item => (item.WithinRange()),
                                           Days1_2 = tempQuery.Count(item => (item.WithinRange(1,2)),
                                           Days3_6 = tempQuery.Count(item => (item.WithinRange(3,6)),
                                           Days7_10 = tempQuery.Count(item => (item.WithinRange(7,10)),
                                           Days11_15 = tempQuery.Count(item => (item.WithinRange(11,15)),
                                           Days16_30 = tempQuery.Count(item => (item.WithinRange(16,30)),
                                           Days31_45 = tempQuery.Count(item => (item.WithinRange(31,45)),
                                           Days46_60 = tempQuery.Count(item => (item.WithinRange(46,60)),
                                           Days60Plus = tempQuery.Count(item => (item.GreaterThan(60));
}.ToList();
share|improve this answer
    
My senior programmer told me to use some instead of count, he claimed that it would make better performance when using on MSSQL Server. – Sarawut Positwinyu Aug 22 '11 at 9:03
2  
It depends on the implementation of specific LINQ provider and he can be right. I posted my comments with assumption that you use standard LINQ to objects. It would better to add this valuable comment to the first post. – Andrey Taptunov Aug 22 '11 at 9:06
    
Actually, i just forgot that SQL don't support using bool so i just need to use tolist, this make my query slower anyway :-P as you said eager evaluation. – Sarawut Positwinyu Aug 22 '11 at 10:27

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.