CODEWITHSUNDEEP
.netc#-4.0
Vasya Pupkin
Vasya Pupkin

Reputation: 645

Foreach loop with Linq statement performance

Witch approach is better, or what is the better way of doing this:

    Stuffs[] stuffs = getStuffs();
   1)

    foreach (var stuff in stuffs.Where(x => x.StartDate <= DateTime.Now.AddDays(-1) && x.EndDate != DateTime.MinValue))
    {
    }

   2) 
    foreach (var stuff in stuffs.Where(x => x.StartDate <= DateTime.Now.AddDays(-1) && x.EndDate != DateTime.MinValue).ToList())
    {
    }

   3)
    stuffs = stuffs.Where(x => x.StartDate <= DateTime.Now.AddDays(-1) && x.EndDate != DateTime.MinValue).ToArray();
    foreach (var stuff in stuffs)
    {

    }

I think number 3 is better for performance. Any more ideas?

Upvotes: 0

Views: 373

Answers (3)

Guffa
Guffa

Reputation: 700192

The first one is generally best for performance. It will filter the data as you loop through it, so it doesn't allocate space for a list or an array to hold all the items.

The second and third perform pretty much the same. They both filter the data and put the result in arrays, and grows the array when needed by copying to a new array.

The third method also replaces the original data with the array of filtered data, which would be relevant if you use the data later on. Also, doing that will free up the original data so that it can be garbage collected if needed, which may be better for performance in the special case when the source array is really huge, and if you are using a lot of memory in the code inside the loop.

If you compare the speed between the options, you won't see much difference. Allocating arrays doesn't take much time, but it will have a small impact later on as they have to be garbage collected eventually.

Upvotes: 0

Joey
Joey

Reputation: 354396

Generally: Measure!

For readability I'd say a hybrid of 1 and 3. With that long a line directly in the header of the foreach the code becomes a little unreadable, so I'd put the query in a separate line above the loop:

var stuffs = from x in getStuffs()
             where x.StartDate <= DateTime.Now.AddDays(-1) &&
                   x.EndDate != DateTime.MinValue
             select x;

or

var stuffs = getStuffs().Where(x => x.StartDate <= DateTime.Now.AddDays(-1) && x.EndDate != DateTime.MinValue);

However you wish. But after that, use the plain foreach loop:

foreach (var s in stuffs) {
}

Also there is no need to convert into a list since foreach can iterate over any iterable collection including LINQ's lazy-evaluated stuff. Converting to the list probably actually costs you time. If you need the query to be evaluated before the loop starts, however, you might need to do it, but that's not exactly a common need (in my experience).

Upvotes: 2

Kobi
Kobi

Reputation: 137997

You should check, but usually getting the data and the action you perform in the loop take more than iterations. The first one should be faster because you aren't creating a new list, which you don't need here.
I'd also suggest placing DateTime.Now.AddDays(-1) in a variable - aside from possible speed benefits, its value may change during the iteration, which may affect the correctness of your program.

Upvotes: 2

Related Questions