22

I'm trying to make some code more readable. For Example foreach(var row in table) {...} rather than foreach(DataRow row in table.Rows) {...}.

To do this I created an extension method:

namespace System.Data {
    public static class MyExtensions {
        public static IEnumerable<DataRow> GetEnumerator( this DataTable tbl ) {
            foreach ( DataRow r in tbl.Rows ) yield return r;
        }
    }
}

But the compiler still throws foreach statement cannot operate on variables of type 'System.Data.DataTable' because 'System.Data.DataTable' does not contain a public definition for 'GetEnumerator'.

To confirm that I implemented the extension method appropriately I tried the following code instead and the compiler had no problem with it.

for ( IEnumerator<DataRow> enm = data.GetEnumerator(); enm.MoveNext(); ) {
    var row = enm.Current;
    ...
}

Before you say that it is because IEnumerator or IEnumerator<DataRow> is not implemented, consider that the following does compile:

public class test {
    public void testMethod() {
        foreach ( var i in new MyList( 1, 'a', this ) ) { }
    }
}
public class MyList {
    private object[] _list;
    public MyList( params object[] list ) { _list = list; }
    public IEnumerator<object> GetEnumerator() { foreach ( var o in _list ) yield return o; }
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
jshall
  • 323
  • 1
  • 2
  • 6
  • 2
    It seems to me that the error message says it all. You need to implement this on the class, not as an extension. – spender Jan 05 '13 at 03:07
  • 1
    I'm confused.. that cannot be the actual code. `for`does not work on enumerator. And `test` isn't an Enumerable. Paste the actual code. – Lews Therin Jan 05 '13 at 03:07
  • 5
    Very subjective note: Making code that looks common (like `var row in table.Rows`) across large number of source files and samples into code that you *feel right now to be nicer* is not necessary the best way of making code more readable. `Foreach` often can be transformed to LINQ statement that is more compact and common enough for people not to question what is happening. Imagine posting your `foreach(var row in table)` in next question on SO - noone will be able to reason about that code without larger sample that includes your magic extensions. – Alexei Levenkov Jan 05 '13 at 03:26
  • 3
    @LewsTherin, by default, `for` does not; however, because he is using it with `GetEnumerator()` it is indeed valid. Not necessarily the most readable, but valid. Also, `foreach` can iterate over any class with a `GetEmumerator()` method with returns an `IEnumerator`. I'm pretty sure that is the actual code. – Steve Konves Jan 05 '13 at 03:26
  • @SteveKonves I failed to see the semicolons xD... Oh yeah That's a new one for me. I thought one had to implement IEnumerator – Lews Therin Jan 05 '13 at 03:32
  • @LewsTherin, I've been messing around with [Sandcastle](http://shfb.codeplex.com) over the last few days, and most of the collection classes in the guts of that project do not implement `IEnumerable`. (which is how I just so happened to find this out.) Interestingly enough, their Enumerator classes don't even implement `IEnumerator`. Each class just has all the right methods, so the compiler goes along with it. It's weird and non very readable, but strangely enough, it works. – Steve Konves Jan 05 '13 at 03:36
  • @SteveKonves thanks for the info. I will be sure to try that out tomorrow. – Lews Therin Jan 05 '13 at 04:18

8 Answers8

44

There is plenty of confusion in the other answers so far. (Though Preston Guillot's answer is pretty good, it does not actually put a finger on what's going on here.) Let me try to clarify.

First off, you are simply out of luck. C# requires that the collection used in a foreach statement either:

  1. Implement a public GetEnumerator that matches the required pattern.
  2. Implement IEnumerable (and of course, IEnumerable<T> requires IEnumerable)
  3. Be dynamic, in which case we simply kick the can down the road and do the analysis at runtime.

The upshot is that the collection type must actually implement the GetEnumerator one way or the other. Providing an extension method does not cut it.

This is unfortunate. In my opinion, when the C# team added extension methods to C# 3 they should have modified existing features such as foreach (and perhaps even using!) to consider extension methods. However, the schedule was extremely tight during the C# 3 release cycle and any extra work items that did not get LINQ implemented on time were likely to be cut. I do not recall precisely what the design team said on this point and I don't have my notes anymore.

This unfortunate situation is the result of the fact that languages grow and evolve; old versions are designed for the needs of their time, and new versions have to build on that foundation. If, counterfactually, C# 1.0 had had extension methods and generics then the foreach loop could have been designed like LINQ: as a simple syntactic transformation. But it was not, and now we are stuck with the legacy of pre-generic, pre-extension-method design.

Second, there seems to be some misinformation in other answers and comments about what precisely is required to make foreach work. You are not required to implement IEnumerable. For more details on this commonly misunderstood feature, see my article on the subject.

Third, there seems to be some question as to whether this behaviour is actually justified by the specification. It is. The specification does not explicitly call out that extension methods are not considered in this case, which is unfortunate. However, the specification is extremely clear on what happens:

The compiler begins by doing a member lookup for GetEnumerator. The member lookup algorithm is documented in detail in section 7.3, and member lookup does not consider extension methods, only actual members. Extension methods are only considered after regular overload resolution has failed, and we haven't gotten to overload resolution yet. (And yes, extension methods are considered by member access, but member access and member lookup are different operations.)

If member lookup fails to find a method group then the attempt to match the pattern fails. The compiler therefore never goes on to the overload resolution portion of the algorithm, and therefore never has a chance to consider extension methods.

Therefore the behaviour you describe is consistent with the specified behaviour.

I advise you to read section 8.8.4 of the specification very carefully if you want to understand precisely how a compiler analyzes a foreach statement.

Fourth, I encourage you to spend your time adding value to your program in some other way. The compelling benefit of

foreach (var row in table)

over

foreach(var row in table.Rows)

is tiny for the developer and invisible to the customer. Spend your time adding new features or fixing bugs or analyzing performance, rather than making already perfectly clear code five characters shorter.

svick
  • 236,525
  • 50
  • 385
  • 514
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
3

The GetEnumerator method in your test class is not static, the extension method is. This doesn't compile either:

class test
{
}

static class x
{
    public static IEnumerator<object> GetEnumerator(this test t) { return null; }
}

class Program
{
    static void Main(string[] args)
    {
        foreach (var i in new test()) {  }
    }
}

In order for the foreach syntax sugar to work your class must expose a public GetEnumerator instance method.

Edit:

As of C# 9.0, GetEnumerator can be an extension method.

Preston Guillot
  • 6,493
  • 3
  • 30
  • 40
  • +1. @jshall, The behavior of picking instance method for iteration is specified in the "8.8.4 The foreach statement" section of "CSharp Language Specification" for C# 4.0. If you read it carefully there is no mentioning of extension methods in search for "GetEnumerator". – Alexei Levenkov Jan 05 '13 at 04:14
  • -1 Extension methods are suppose to be static (see http://msdn.microsoft.com/en-us/library/bb311042.aspx ). Also `Extension methods enable you to "add" methods to existing types without creating a new derived type, recompiling, or otherwise modifying the original type.` (see http://msdn.microsoft.com/en-us/library/vstudio/bb383977.aspx) – Trisped Jan 05 '13 at 04:38
  • Of course extension methods are static. That's the point, GetEnumerator can't be static. – Preston Guillot Jan 05 '13 at 04:46
  • I think Alexei is on to something. What I am unsure of is whether checking for GetEnumerator as an extension method is an intentional omission or an oversight. All the documentation I've read implies that it should work, but the compiler seems reflect the specification's omission. – jshall Jan 05 '13 at 05:56
  • 1
    "Perform overload resolution using the resulting method group and an empty argument list. If overload resolution results in no applicable methods, results in an ambiguity, or results in a single best method but that method is either static or not public, check for an enumerable interface as described below." The spec calls out static implementations of GetEnumerator, and extension methods are static. – Preston Guillot Jan 05 '13 at 05:59
  • @PrestonGuillot: Regarding your last comment: you are very close, but have not *quite* put your finger on it. The compiler never reaches this step because the "resulting method group" does not exist in the first place! Member lookup does not find extension methods, so the method group never exists in the first place. – Eric Lippert Jan 05 '13 at 14:45
2

This will be possible in C# 9. The proposal is already checked in. You can verify in Language Feature Status - C# 9 when it will become available in a preview version.

In the detailed design section of the proposal, it says:

Otherwise, determine whether the type 'X' has an appropriate GetEnumerator extension method

using System;
using System.Collections.Generic;

public static class MyExtensions
{
    // Note: ranges aren't intended to work like this. It's just an example.
    public static IEnumerable<int> GetEnumerator(this Range range)
    {
        // .. do validation ..
        for (var i = range.Start.Value; i <= range.End.Value; i++)
        {
            yield return i;
        }
    }
}

public class ExtensionGetEnumerator
{

    public void Method()
    {
        var range = 1..2;
        foreach (var i in range.GetEnumerator())
        {
            Console.WriteLine($"Print with explicit GetEnumerator {i}");
        }

        // The feature is in progress, scheduled for C# 9, the below does not compile yet

        //foreach (var i in range)
        //{
        //    Console.WriteLine($"Print with implicit GetEnumerator {i}");
        //}
    }
}
Andrei Epure
  • 1,746
  • 1
  • 21
  • 30
0

some offtopic: if you want to do it more readable write

foreach ( DataRow r in tbl.Rows ) yield return r;

as

foreach (DataRow row in tbl.Rows) 
{
    yield return row;
}

now to your problem.. try this

    public static IEnumerable<T> GetEnumerator<T>(this DataTable table)
    {
        return table.Rows.Cast<T>();
    }
Lucas
  • 3,376
  • 6
  • 31
  • 46
  • 4
    I would say your first point is a matter of opinion, not fact. – Ed S. Jan 05 '13 at 03:22
  • There is no problem with the implementation of GetEnumerator, the compiler simply fails to find it. – jshall Jan 05 '13 at 04:12
  • 1
    @jshall, it does not "fail to find", it "must not find" extension method, - see my other comment on location of this requirement in the C# specification. – Alexei Levenkov Jan 05 '13 at 04:16
0

Your extension is equivalent to:

    public static IEnumerable<TDataRow> GetEnumerator<TDataRow>( this DataTable tbl ) {
        foreach ( TDataRow r in tbl.Rows ) yield return r;
    }

GetEnumerator<TDataRow> is not the same method as GetEnumerator

This will work better:

    public static IEnumerable<DataRow> GetEnumerator( this DataTable tbl ) {
        foreach (DataRow r in tbl.Rows ) yield return r;
    }
Amy B
  • 108,202
  • 21
  • 135
  • 185
  • 1
    That may "work better" for resolving the `var` in `foreach(var row in table)` but it still does not compile. – jshall Jan 05 '13 at 03:56
0

Within an foreach statement the compiler is looking for an instance method of GetEnumerator. Therefore the type (here DataTable) must implement IEnumerable. It will never find your extension method instead, because it is static. You have to write the name of your extension method in the foreach.

namespace System.Data {
    public static class MyExtensions {
        public static IEnumerable<DataRow> GetEnumerator( this DataTable table ) {
            foreach ( DataRow r in table.Rows ) yield return r;
        }
    }
}

foreach(DataRow row in table.GetEnumerator())
  .....

To avoid confusion, I would suggest using a different name for your extension method. Maybe something like GetRows()

Matthias
  • 5,574
  • 8
  • 61
  • 121
0

Current proposal to add GetEnumerator via Extension to C# https://github.com/dotnet/csharplang/issues/3194

Ben Adams
  • 3,281
  • 23
  • 26
-6

The object collection in a foreach must implement System.Collections.IEnumerable or System.Collections.Generic.IEnumerable<T>.

If you have a very strong desire to enable this then you could create a wrapper class which implements IEnumerable and has a pointer to you DataTable. Alternatively, you could inherit DataTable in a new class and implement IEnumerable.

Code readability is most often personal preference. I personally find your changes to the foreach statement less readable (but I am sure there are many on SO who agree with you).

Trisped
  • 5,705
  • 2
  • 45
  • 58
  • See the end of the question, inheritance is not necessary and the last bit of code proves it. – jshall Jan 05 '13 at 05:00
  • @jshall Seems you missed the links to the documentation in my answer. It is necessarily. Just because it compiles does not mean it is going to run, as you have already proven. If you are really that interested in using a `DataTable` as the object collection in a `foreach` statement then you will need to either use a wrapper class as I suggested or inherit `DataTable` in a new class which implements `IEnumerable`. – Trisped Jan 05 '13 at 05:12
  • @jshall I guess I should add the reason your second example works is because the compiler adds the interface to the class for you. Since it is not compiling the `DataTable` class it can not add the interface in your problem. The other issue with your example is that it is not using an extension to add the method. I doubt that it would compile if you were. – Trisped Jan 05 '13 at 05:15
  • 2
    The documentation at your link is incomplete. The C# Language Spec clarifies that implementing IEnumerable or IEnumerable is not necessary for foreach to function. In fact, the compiler looks for the interface implementation last, after checking if the type is an array, a dynamic, or provides a sufficient instance method named GetEnumerator. http://msdn.microsoft.com/en-us/library/ms228593.aspx see section 8.8.4 – Preston Guillot Jan 05 '13 at 05:29
  • @PrestonGuillot it seems you have confused the compiler with the language. As I noted, the compiler will add the interface if needed, which is why it checks for arrays, dynamic, or an implementation of GetNumerator (which returns an IEnumerator). This would be similar to many other tools which create or alter code without the user knowing. – Trisped Jan 05 '13 at 06:13
  • @Trisped go to http://msdn.microsoft.com/en-us/library/vstudio/9yb8xew9.aspx and read the paragraph following the first example. – jshall Jan 05 '13 at 06:25
  • @Trisped The compiler never adds an interface implementation to a class to generate CIL for a foreach loop. See http://pastebin.com/Vrrv51cH. The compiler generates CIL that, as specified by the language spec, is logically equivalent to invoking GetEnumerator() on the class, then MoveNext() and Current on the resulting IEnumerator. In the case of arrays this logical equivalence is generated by evaluating all positions of the array in order, instead of actual calls to an IEnumerable, but the CIL is equivalent for a class that defines GetEnumerator, one that implments IEnumerable and dynamic. – Preston Guillot Jan 05 '13 at 07:22
  • 3
    Trisped: your analysis is incorrect. @PrestonGuillot's analysis is correct. – Eric Lippert Jan 05 '13 at 14:24