Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Explicit implementation of interface's GetEnumerator causes stack overflow

Tags:

c#

I'm doing my best to code against interfaces whenever possible, but I'm having some issues when it comes to collections. For example, here are a couple interfaces I'd like to use.

public interface IThing {}

public interface IThings : IEnumerable<IThing> {}

Here are the implementations. In order to implement IEnumerable<IThing> I need to explicitly implement IEnumerable<IThing>.GetEnumerator() in Things.

public class Thing : IThing {}

public class Things : List<Thing>, IThings
{
    IEnumerator<IThing> IEnumerable<IThing>.GetEnumerator()
    {
        // This calls itself over and over
        return this.Cast<IThing>().GetEnumerator();
    }
}

The problem is that the GetEnumerator implementation causes a stack overflow. It calls itself over and over again. I can't figure out why it'd decide to call that implementation of GetEnumerator instead of the implementation provided by the result of this.Cast<IThing>(). Any ideas what I'm doing wrong? I'm willing to bet it's something extremely silly...

Here's some simple test code for the above classes:

static void Enumerate(IThings things)
{
    foreach (IThing thing in things)
    {
        Console.WriteLine("You'll never get here.");
    }
}

static void Main()
{
    Things things = new Things();
    things.Add(new Thing());

    Enumerate(things);
}
like image 591
Joe Avatar asked Nov 05 '09 18:11

Joe


3 Answers

Use this instead:

    public class Things : List<Thing>, IThings
    {
        IEnumerator<IThing> IEnumerable<IThing>.GetEnumerator()
        {
            foreach (Thing t in this)
            {
                yield return t;
            }
        }
    }

Or you could work with containment instead.

like image 137
Stormenet Avatar answered Sep 29 '22 11:09

Stormenet


IEnumerator<IThing> IEnumerable<IThing>.GetEnumerator()
{
    // This calls itself over and over
    return this.Cast<IThing>().GetEnumerator();
}

It's a recursive call with no break condition, you're going to get a stack overflow, because it's going to very quickly fill up the stack.

You're casting to an interface, which does not do what you think it does. In the end you are just calling this.GetEnumerator(); which is the function you're already in, thus recursing. Perhaps you mean base.GetEnumerator();

like image 37
Russell Steen Avatar answered Sep 29 '22 11:09

Russell Steen


Generally, you would probably want to decouple the implementation of the Things collection from the implementation of the Thing object. The Things class is capable of working with any implementation of IThing, not just the Thing class.

Specifically:

public class Things : List<IThing>, IThings
{
}

In this case, you don't have to override the default implementation of GetEnumerator(), the base implementation is already typed correctly for you. This will avoid the overflow that you're currently experiencing and satisfies the test case you provided.

like image 32
Kyle Chafin Avatar answered Sep 29 '22 11:09

Kyle Chafin