Re: Linq query vs foreach loop - C# Discussion Boards

9-Feb-08 1:13

I'm loving the Linq stuff, it's very cool and flexible, but I don't want to use is for the sake of it.

I currently have a Generic List of DVD objects that have Name and ID properties.
I'm writing a simple method in the List to return the Name of the DVD given an ID and return an empty string if it's not found.

Before I was doing

public string GetName(int ID)
{
    foreach (DVD thisDVD in this)
    {
        if (thisDVD.ID == ID)
        { return thisDVD.Name; }
    }
    return "";
}

which works fine, but after playing with Linq, I found myself writing

public string GetName(int ID)
{
    var idQuery =
        from dvd in this
        where dvd.ID == ID
        select dvd.Name;
    foreach (string name in idQuery)
    { return name; }
    return "";
}

which works fine too.

I was wondering which to use? I figured that the first one was better, as in the second Linq is iterating over the collection and then I have to iterate over the results. Surely only one trip over and breaking out if/when the required result is found is better? Is there something in the framework that makes Linq more efficient?

Dave

modified on Saturday, February 9, 2008 7:25 AM

^That's Aragon9-Feb-08 1:30

^That's Aragon

9-Feb-08 1:30

Hi Dave!

Look here[^]

Regards.

"Save water,It's precious" Smile | :)

"Don't forget to vote" Wink | ;)

Daniel Grunwald9-Feb-08 2:27

Daniel Grunwald

9-Feb-08 2:27

LINQ is a bit slower due to the overhead of tons of method calls (there are lots of calls to MoveNext() and the closure). The current JIT is not able to optimize this, but it's possible to write an IL-level optimizer that inlines all LINQ method calls, then inlines the closure method calls and removes the closure objects, converting the fields representing captured variables back into local variables.
I've written a prototype optimizer that can remove the closure in code like this:

static List<int> someIntList = new List<int>();

static bool ExistsTest(int divisor)
{
	return Exists(someIntList, i => i % divisor == 0);
}

static bool Exists(IEnumerable<int> input, Predicate<int> pred)
{
	if (pred == null) {
		throw new ArgumentNullException("pred");
	}
	foreach (int i in input) {
		if (pred(i))
			return true;
	}
	return false;
}</int></int></int></int>

What's missing until it gets useful is cross-assembly inlining and support for generic methods.

DaveyM699-Feb-08 3:04

9-Feb-08 3:04

Thanks cosmo - that's been very helpful

Dave

Gareth H9-Feb-08 1:31

Gareth H

9-Feb-08 1:31

DaveyM69,

Since your using a List<t> why not do:

return list[list.IndexOf(ID)];

And you could always add list.Containts(ID) before the above to check its in the list so you dont get an exception. It just seems simpler/quicker than iterating.

Regards,
Gareth.

Daniel Grunwald9-Feb-08 2:36

Daniel Grunwald

9-Feb-08 2:36

I would write it this way:

public string GetName(int ID)
{
    return
       (from dvd in this
        where dvd.ID == ID
        select dvd.Name
       ).FirstOrDefault() ?? "";
}

If performance of GetName is an issue, neither a loop nor LINQ is efficient; you should use a Dictionary ID->dvd.

DaveyM699-Feb-08 3:03

9-Feb-08 3:03

Daniel Grunwald wrote:
FirstOrDefault()

I was just looking at the Element Operations on MSDN Smile | :)

I've tried various ways of coding it and Linq is definately slower in ALL situations (it is more 'elegant' though) by between 30 and 50%.

Performance isn't really an issue in this app but I didn't want to deliberately add overhead.

Thanks for all your help and suggestions everyone!

Dave

WillemM9-Feb-08 7:54

WillemM

9-Feb-08 7:54

One of the reasons that you would use LINQ is that it allows you to write simpler code.
You query for the DVD could be a lot shorter.

public string GetName(int ID)
{
return this.Where(d => d.ID == id).Select(d => d.Name).FirstOrDefault() ?? String.Empty;
}

This is acually quite efficient, as the linq operator doesn't copy any data, but uses chained yield return methods to find the required subset of dvd's. So Link is actually just as efficient as your code would be, with the added advantage of a simple query language.

WM.

What about weapons of mass-construction?
"What? Its an Apple MacBook Pro. They are sexy!" - Paul Watson
My blog

DaveyM699-Feb-08 8:29

9-Feb-08 8:29

I've just tested it on one million records searching for the id 999999 and it still performs about 30 to 50% slower than a foreach loop.

It seems that regardless of what you atually try to do with a Linq query it calls some sort of internal ToArray() method (so the result supports the IEnumerable interface I assume) and that's where the performance is lost.

For complex queries I think Linq has some huge advantages, but for simple ones it seems the old foreach is better.

Dave

Guffa9-Feb-08 8:30

Guffa

9-Feb-08 8:30

Why not simpy:

public string GetName(int ID) {
   DVD found = this.Find(d => d.ID == ID);
   return found != null ? found.Name : string.Empty;
}

But, as Daniel said, if performance is an issue, you should use a dictionary instead of a list.

Experience is the sum of all the mistakes you have done.

DaveyM699-Feb-08 8:44

9-Feb-08 8:44

The dictionary would be fine if a 'real world' class were actually as simple as the example. If matching multiple properties then it's not much use.

That code performs 10 to 20% quicker than a foreach! Cool, thankyou! Big Grin | :-D

Dave

Guffa9-Feb-08 11:40

Guffa

9-Feb-08 11:40

DaveyM69 wrote:
f matching multiple properties then it's not much use.

Then just make a key that contains multiple properties. Make a class that implements IEqualityComparer and use that as comparer when you create the dictionary.

DaveyM69 wrote:
That code performs 10 to 20% quicker than a foreach!

Hm... that's pretty strange, as all the Find method does is looping through the items and calling the delegate...

A dictionary would perform about 100000% faster...

Experience is the sum of all the mistakes you have done.

Guffa13-Feb-08 8:19

Guffa

13-Feb-08 8:19

DaveyM69 wrote:
If matching multiple properties then it's not much use.

Regarding that, I wrote an article about how to make a custom key for a dictionary:

Dictionary with a custom key[^]

Experience is the sum of all the mistakes you have done.

Document scanning in C#

Member 26713148-Feb-08 23:27

Member 2671314

8-Feb-08 23:27

Hi,Please tel me how I can scan a document in c#.thanks.

Chamanara Shapoor

Gareth H9-Feb-08 1:33

Gareth H

9-Feb-08 1:33

Member 2671314,

What do you mean by "scan a document"?

Regards,
Gareth.

Thomas Stockwell9-Feb-08 2:49

Thomas Stockwell

9-Feb-08 2:49

I would assume you want to loop through the text of the entire document for single terms/phrases. Just use a for loop or while(!stream.endoffile).

Regards,
Thomas Stockwell

Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.

Visit my homepage Oracle Studios[^]

Pete O'Hanlon9-Feb-08 9:27

Pete O'Hanlon

9-Feb-08 9:27

Buy an OCR toolkit and hook it in, or look at writing a TWAIN class. There should be some samples floating around that you could work from.

Deja View - the feeling that you've seen this post before.

My blog | My articles