Method Take iterates over all elements

[GoogleCodeExporter]

Problem: 
Method Take<TSource>(int count) (line 631) iterates over all elements of 
source, because it uses method Where() in implementation: source.Where((item, 
i) => i < count). It isn't efficient.

Proposed implementation:
if (count > 0) {
  foreach (var item in source){
    yield return item;
    if (--count == 0) {
      break;
    }
  }
}

[Test]
public void Take_IsLazy()
{
    TakeTestLazy.InstancesCount = 0;
    var taked = TakeTestLazy.GetFiveInstances().Take(0).ToArray();
    CollectionAssert.IsEmpty(taked);
    Assert.AreEqual(0, TakeTestLazy.InstancesCount);
    taked = TakeTestLazy.GetFiveInstances().Take(3).ToArray();
    Assert.AreEqual(3, taked.Length);
    Assert.AreEqual(3, TakeTestLazy.InstancesCount);
}

private class TakeTestLazy {
   [ThreadStatic]
   public static int InstancesCount;

   public TakeTestLazy() {
      InstancesCount++;
   }

   public static IEnumerable<TakeTestLazy> GetFiveInstances() {
      for(int i=0; i<5; i++) { yield return new TakeTestLazy(); }
   }
}

Original issue reported on code.google.com by alec.che...@gmail.com on 17 Nov 2010 at 11:40

[GoogleCodeExporter]

The goal of LINQBridge release 1.1 was correctness & coverage over 
optimization. Many operators, like Take, are implemented in terms of more basic 
operators, like Where, to attain 100% coverage. Since base operators were well 
covered, the idea was that if most operators could be implemented in terms of 
the base ones then the coverage goal would be attained with fewer tests. 
Optimizations are certainly welcome and many operators could be implemented 
more efficiently but it would help tremendously if you could submit a patch 
with suggested implementation along with tests that guarantee continued full 
coverage.

BTW, Take_IsLazy test does not test for laziness. For example, Take could be 
written for eager evaluation and Take_IsLazy would still pass. Also, you would 
need code and tests to check arguments eagerly but evaluate lazily.

Original comment by azizatif on 18 Nov 2010 at 11:15

• Changed state: Accepted
• Added labels: Performance

[GoogleCodeExporter]

It's not an optimization issue, but the real bug: Take will never complete 
enumeration over infinite enumerables.

So, I'm fixing it locally with the following patch.

Original comment by mrdont@mail.ru on 15 Jan 2011 at 4:16

Attachments:

• Enumerable-Take.patch

[GoogleCodeExporter]

Skip(count) could also be implemented with SkipWhile passing it inverted 
predicate, i.e. i < count.

Original comment by mrdont@mail.ru on 15 Jan 2011 at 4:27

[GoogleCodeExporter]

Original comment by azizatif on 16 Jan 2011 at 11:57

• Added labels: Milestone-Release1.3

[GoogleCodeExporter]

Fixed in r248.

Original comment by azizatif on 17 Jan 2011 at 6:53

• Changed state: Fixed