Iterating through a collection of objects

Question

Subject

I have to iterate through a collection of objects (in this case IGrouping<string, SupplyDemand>). So I could do this easyly using a query with LINQ.

Code

 public IEnumerable<string> getParentPegs(IGrouping<string, SupplyDemand> data)
 {
 IEnumerable<string> query =
 from d in data
 where d.Source.ToLower() == "make"
 select d.Part;
 return query;
 }

Problem

I'm using this function inside a loop in a VSTO Excel project and it's pretty slow, besides it stops running (memory leak). So I came up with the following code:

Improved Code

 public List<string> getParentPegs(IGrouping<string, SupplyDemand> data)
 {
 List<string> result = new List<string>();
 var enumerator = data.GetEnumerator();
 while (enumerator.MoveNext())
 {
 SupplyDemand obj = enumerator.Current;
 if (obj.Source.ToLower() == "make")
 {
 result.Add(obj.Peg);
 }
 }
 return result;
 }

Question

Is there any other way to improve this?

• Makotosan: How are you using the getParentPegs method? The original code returns a lazily evaluated expression and not an actual collection, so if in your code you do this: var pegs = getParentPegs(....); int count = pegs.Count(); foreach(var peg in pegs){ // Doing something else here } Will cause your expression to be evaluated twice, whereas the improved code will only evaluate it once. You can overcome this by forcing it to evaluate and store it in a list by calling the .ToList() method: var pegs = getParentPegs(...).ToList(); Or return a List within getParentPegs.
• Eder: Makotosan; I'm using it inside a loop, that's what I've been trying to optimize (archiving all parent pegs for a component). Yes, you're right about evaluating the expression twice, so I came up with the second code. So mainly, this question is about the linq performance against a simple iteration.

Answer 1

You can do the same with just a foreach loop instead of reading the enumerator.

You can use the String.Compare method to do a case insensetive compare instead of creating a lowercase version of each string that you compare.

Improved improved code:

public List<string> getParentPegs(IGrouping<string, SupplyDemand> data) {
 List<string> result = new List<string>();
 foreach (SupplyDemand obj in data) {
 if (String.Compare(obj.Source, "make", true) == 0) {
 result.Add(obj.Peg);
 }
 }
 return result;
}

Note: In your first code you are getting the property Part, but in the second you are getting the property Peg instead.

Answer 2

how about this :

 public IEnumerable<string> GetParentPegs(IGrouping<string, SupplyDemand> data)
 {
 foreach(var d in data)
 if(d.Source.ToLower() == "make") yield return d.Part;
 }

• Eder: Excellent!, But how come that this is faster than my second example?
• Beatles1692: It actually does the same thing but it's a bit more readable in my opinion
• Olivier Jacot-Descombes: It might be faster if you are not evaluating the result of GetParentPegs immediately, since the loop in GetParentPegs will only be executed when the IEnumerable<string> is enumerated.