Linq query - Tidy Up/Optimization

Question

I am using the following line of code:

ListOfMyClass.OrderByDescending(x => x.DateCreated)
             .Where(x => !x.Deleted).First();

And I get

"Replace with single call to first warning"

Which I would prefer to do but cant work out how.

I want to be able to say for the item with the Latest datecreated and that is marked as not deleted.

Where's the best place to learn these kind of query's and what's the best way to solve this one as I prefer to be a tidy coder.

What is this: `Replace with single call to first warning` ? I doubt that this is the exact warning. Apart from that, what tool throws that warning? — Tim Schmelter, Oct 01 '13 at 09:01

Kamil Budziewski · Accepted Answer · 2013-10-01T09:12:44.000

6

You can call it without where:

ListOfMyClass.OrderByDescending(x => x.DateCreated).First(x => !x.Deleted);

First accepts predicates also. It really does not matter if you write Where and First in this case or just First it's evaluated to the same query at all :) But First looks cleaner.

As Jeroen van Langen said Where is slightly faster than First. So I assume that using Where.OrderByDescending.First is the best solution so far.

edited Oct 01 '13 at 09:12

answered Oct 01 '13 at 08:59

Kamil Budziewski

22,699
14
85
105

I thought it ment order by descending isnt required as you can pull the newest date value using first it was just how to do that i was stuck? – Lemex Oct 01 '13 at 09:01
1

@LmC single call to first warning means that you can replace `Where.First` with just `First` :) – Kamil Budziewski Oct 01 '13 at 09:02
3

I wouldn't do the Where in the `First()`. Look here for more info: http://stackoverflow.com/questions/8663897/why-is-linq-wherepredicate-first-faster-than-firstpredicate – Jeroen van Langen Oct 01 '13 at 09:05

score 5 · Answer 2 · answered Oct 01 '13 at 08:59

5

It's just a suggestion to use the other overload of First, which accepts an inline predicate. This allows you to replace .Where(...).First() with just .First(...):

ListOfMyClass.OrderByDescending(x => x.DateCreated).First(x => !x.Deleted);

That said, an even better way to write the query would be

ListOfMyClass.Where(x => !x.Deleted)
             .OrderByDescending(x => x.DateCreated)
             .First();

Here the above "code-writing optimization" does not apply, but this version filters before sorting. This could speed up the sort substantially, depending on what percentage of items is filtered out. There really is no sense in sorting elements that you are just going to disregard.

answered Oct 01 '13 at 08:59

Jon

428,835
81
738
806

I thought it ment order by descending isnt required as you can pull the newest date value using first it was just how to do that i was stuck? – Lemex Oct 01 '13 at 09:00
1

@LmC: No, the order is required. – Jon Oct 01 '13 at 09:02
I think using the Where before ordering is the way to go. +1 – Jeroen van Langen Oct 01 '13 at 09:04

score 2 · Answer 3 · answered Oct 01 '13 at 09:00

2

To 'satisfy' the warning you could rewrite it to:

ListOfMyClass.OrderByDescending(x => x.DateCreated)
             .First(x => !x.Deleted);

But I prefer to do filtering before sorting, so I'd use this:

ListOfMyClass
             .Where(x => !x.Deleted)
             .OrderByDescending(x => x.DateCreated)
             .First();

answered Oct 01 '13 at 09:00

Jakub

139
5

Is there any benifit to filter before sorting – Lemex Oct 01 '13 at 09:01
@LmC, ignore my previous statement. If this is Linq to objects (which I'm presuming it is) the ordering of the where, etc. is relevant [see here](http://stackoverflow.com/a/6802942/542251) – Liam Oct 01 '13 at 09:07

Henrik · Answer 4 · 2013-10-01T09:16:40.110

1

I'd put the Where first, so there are less items to sort.

ListOfMyClass.Where(x => !x.Deleted).OrderByDescending(x => x.DateCreated).First();

Also, you might want to use MaxBy fom MoreLinq:

ListOfMyClass.Where(x => !x.Deleted).MaxBy(x => x.DateCreated);

edited Oct 01 '13 at 09:16

answered Oct 01 '13 at 09:02

Henrik

23,186
6
42
92

@Liam I'm not sure. I would assume `OrderByDescending` reads all items and sorts them in O(nlogn). – Henrik Oct 01 '13 at 09:06
I think it depends. If it's *Linq to Objects* your right, if it's *Linq to SQL* the lazy loading will create a single SQL statement and this will have no effect. I'm guessing this is Linq to objects so it is relevant. – Liam Oct 01 '13 at 09:09

score 0 · Answer 5 · answered Oct 01 '13 at 09:03

0

ListOfMyClass.Where(x=> !x.Deleted).OrderByDescending(x=>x.DateCreated).First();

answered Oct 01 '13 at 09:03

Derek

8,300
12
56
88

score 0 · Answer 6 · answered Oct 01 '13 at 09:04

0

...the latest datecreated and that is marked as deleted...

If you want the latest, deleted item, why are you asking for non-deleted(!x.Deleted)?

I would suggest this:

var yourClassOrNull = ListOfMyClass
    .Where(x => x.Deleted)
    .OrderByDescending(x => x.DateCreated)
    .FirstOrDefault();

This is readable, safe and efficient. It is safe because FirstOrDefault returns the first item that matches the condition in the Where ordered by OrderByDescending. If no items match the condition the default value is returned, in case of reference types like here null. So you can check later if(yourClassOrNull != null) ....

It is efficient since it's important to order after you have filtered in Linq-To-Objects.

answered Oct 01 '13 at 09:04

Tim Schmelter

450,073
74
686
939

I consider `OrderByDescending` followed by `First`(`OrDefault`) neither readable nor efficient. That's why there is a `MaxBy` extension method in MoreLinq. – Henrik Oct 01 '13 at 09:33
@Henrik: It's as readable as sql is. And `MaxBy` is more efficient, but it wansn't even clear if it's `Linq-To-Objects` at all and i've assumed that he wants an answer with .NET methods only. Also, if `.Where(x => x.Deleted)` retuns only few items it's not expensive to order them. – Tim Schmelter Oct 01 '13 at 09:50

Linq query - Tidy Up/Optimization

6 Answers6