11

I have a class TaskWeekUI with this definition:

 public class TaskWeekUI    {
   public Guid TaskWeekId { get; set; }
   public Guid TaskId { get; set; }
   public Guid WeekId { get; set; }
   public DateTime EndDate { get; set; }
   public string PersianEndDate { get; set; }
   public double PlanProgress { get; set; }
   public double ActualProgress { get; set; }    } 

and I wrote this query :

 TaskWeekUI ti =  tis.First( t => t.PlanProgress > 0 && t.EndDate ==  tis.Where(p => p.PlanProgress != null && p.PlanProgress > 0).Max(w => w.EndDate));

Is this query is true? Can I write my query better than this?

Shayan
  • 282
  • 1
  • 3
  • 18

4 Answers4

36

I think you want the one whose PlanProgress > 0 has a most recent EndDate.

TaskWeekUI ti = tis.Where(t => t.PlanProgress > 0)
                   .OrderByDescending(t => t.EndDate)
                   .FirstOrDefault();
Cheng Chen
  • 39,413
  • 15
  • 105
  • 159
  • 1
    +1, nice clear solution without any extraneous exceptions introduced by using Max. – WileCau Dec 31 '10 at 02:44
  • 1
    Tnx,I think it is a best any simple solution . – Shayan Jan 01 '11 at 07:50
  • 2
    Isn't it a waste of time to sort the whole list just to get the item with the highest `EndDate`? [This](http://stackoverflow.com/a/1101979/1219414) seems to be a better solution. – Juan Feb 25 '14 at 19:03
  • 1
    @JuanLuisSoldi: You are correct. The best solution should be O(n). But anyway it's acceptable, and it's the best solution base on build-in LINQ functions. – Cheng Chen Mar 02 '14 at 15:50
4

This query seems to be correct from point of view of result obtained.

But in your inner query tis.Where(p => p.PlanProgress != null && p.PlanProgress > 0).Max(w => w.EndDate) is computed for each element in collection with t.PlanProgress > 0

So its a better way to obtain Max value outside of a query as follows:

var max = tis.Where(p => p.PlanProgress != null && p.PlanProgress > 0).Max(w => w.EndDate);
tis.First( t => t.PlanProgress > 0 && t.EndDate == max);

Going further p.PlanProgress != null is allways true since p.PlanProgress is not of Nullable type. So our code becomes like this:

var max = tis.Where(p => p.PlanProgress > 0).Max(w => w.EndDate);
    tis.First( t => t.PlanProgress > 0 && t.EndDate == max);

Or you can change a definition of your class and make p.PlanProgress of Nullable type:

public class TaskWeekUI    {
   public Guid TaskWeekId { get; set; }
   public Guid TaskId { get; set; }
   public Guid WeekId { get; set; }
   public DateTime EndDate { get; set; }
   public string PersianEndDate { get; set; }
   public double? PlanProgress { get; set; }
   public double ActualProgress { get; set; }    
} 

var max = tis.Where(p => p.PlanProgress.HasValue && p.PlanProgress.Value > 0).Max(w => w.EndDate);
    tis.First( t => t.PlanProgress.HasValue && t.PlanProgress.Value > 0 && t.EndDate == max);
ILya
  • 2,552
  • 4
  • 25
  • 40
  • 1
    +1, the only thing I would change is the `First` to a `FirstOrDefault` unless he can guarantee that there will always be an item with `PlanProgress > 0` – Matthew Abbott Dec 30 '10 at 11:37
  • +1, but I think 'collection.Max' throws an exception if 'collection' is empty, so there probably needs to be an additional check before setting 'max'. The collection will be empty if all PlanProgress <= 0. Since there are checks for PlanProgress > 0 I assume PlanProgress <= 0 is legal and an exception probably wouldn't be expected. – WileCau Dec 30 '10 at 12:20
0
     var max_Query =
                   (from s in db.Songs
                    join bk in db.Albums on s.BookId equals addAlbumDetailsViewModel.BookId
                    select s.SongId).Max();
                            max_Query++;
-1

You don't need compare PlanProgress with null because double is struct type, it can't be null.

If you want TaskWeekUI with Max EndDate and positive PlanProgress You can try this code:

TaskWeekUI ti = tis.Where(t => t.PlanProgress > 0).Max(w => w.EndDate);
Eugene Gluhotorenko
  • 2,894
  • 2
  • 29
  • 51