Override IEnumerable in More Derived Class - Any Potential Issues?

Question

I have a abstract, custom collection type we'lll call AnimalCollection for storing a collection of Animal objects. Animals that need to be stored in an animal collection are so special that they get their own implementation of AnimalCollection for storing additional information specific to that type of collection, e.g. TurtleCollection, GiraffeCollection, etc.

Since we're all collections here, I think it makes sense that AnimalCollection should implement IEnumerable<Animal> so users can iterate over it. However, in the case of our more derived classes it makes more sense to have implemented IEnumerable<Turtle>, IEnumerable<Giraffe>, etc.

class Animal {}
class Turtle : Animal {}

abstract class AnimalCollection : IEnumerable<Animal>
{
}

class TurtleCollection : AnimalCollection, IEnumerable<Turtle>
{
}

That way, when someone does foreach (var turtle in turtleCollection) var is automatically of type Turtle. Without having implemented IEnumerable<Turtle>, the user would have had to type foreach (Turtle turtle in turtleCollection). Since I always type var everywhere I can, I'm sure I would eventually trip over the fact my turtle variable wasn't casted to the type I expected, which means the users of my API would probably stumble over this as well!

So while I'm aware that having a type implement IEnumerable<T> on two adjacent types in an inheritance tree can cause issues, I'm wondering whether there could be hidden gotchas in using multiple IEnumerable<T> with types in the same inheritance hierarchy?

---

EDIT: The most sensible solution here would be to have AnimalCollection<T> and then define TurtleCollection : AnimalCollection<Turtle> as has been pointed out by some of the existing comments/answers. However the reason my AnimalCollection is non-generic is that the type I'm calling AnimalCollection actually represents a node in a tree. In actuality, I have types like

class Node {}
class FooNode : Node {}
class BarNode : Node {}

class NodeCollection : Node {}
class FooNodeCollection : NodeCollection {}
class BarNodeCollection : NodeCollection {}

FooNodeCollection is constrained in its constructor to only take objects of type FooNode. I then have visitors for accessing the nodes of a tree

class NodeVisitor
{
    abstract void VisitFoo(FooNode foo);
    abstract void VisitBar(BarNode bar);
    abstract void VisitCollection(NodeCollection collection);
}

The exact type of node that is contained in the NodeCollection doesn't really matter as far as visitors are concerned; all the visitor will do is simply iterate over its children and have them dispatch themselves to the visitor.

Potentially one solution then could be to have two types - NodeCollection which is used by the visitor, and NodeCollection<T> which derives from NodeCollection and is the one that actually implements IEnumerable<T>, however I'm also interested to know what types of issues could arise from utilizing multiple IEnumerable<T> in the way originally stated

Answer 1

I'd structure the code to use composition, not inheritance.

class Animal {}
class Turtle : Animal { public Color ShellColor {get;set;} }

abstract class AnimalCollection<T> where T : Animal
{
   public IEnumerable<T> Animals {get; set;} 
}

class TurtleCollection : AnimalCollection<Turtle> {}

(...)
foreach( var t in new TurtleCollection().Animals)
{
    Console.WriteLine(t.ShellColor);
}

---

(Update)

Oh, yeah, Why not inherit from List<T>? is always a good read as well.

Answer 2

An additional answer to the question you linked to also hints at why multiple implementation of IEnumerable<T> for types in the same inheritance hierarchy would be bad. If you consider the scenario of the answer, trying to cast to the base type would leave the runtime making an undefined choice, which is something that you should, to quote the answer: Avoid, avoid, avoid.

Beyond that, let me remind you of OfType<T>:

foreach (var turtle in collection.OfType<Turtle>())
{
    //turtle is guaranteed to be of type Turtle.
    //If no Turtle objects exist in the collection,
    //the iteration will simply not execute!
}

And, finally, your posted code would definitely be clearer if you followed the suggestion of Federico Dipuma:

Just create a class AnimalCollection<TAnimal> : IEnumerable<TAnimal> where TAnimal : Animal.

EDIT

Based on the updated question, my understanding is that the visitor does something along these lines:

public void VisitCollection(NodeCollection node)
{
    //The problem is that, here, there is no way to know
    //(without pattern-matching or type checks in general)
    //what node type the collection contains. However
    //it may not matter at all, so there should be no problem.
}

Your problem would arise wherever you would be coding with access to both IEnumerable<T> implementations, i.e. wherever you have access to the specific class type rather than the base type. The VisitCollection method is certainly not one of these cases, because within the scope of the method, you only know (cf. see) an IEnumerable<Node> and not an IEnumerable<FooNode> or otherwise. Therefore, you are not "in danger" inside this method.

If, however, your visitor needs to know the specific node type in case of collections, you would have:

public abstract class Visitor
{
    //As in your case...
    abstract void VisitFooNodeCollection(FooNodeCollection collection);
    abstract void VisitBarNodeCollection(BarNodeCollection collection);
}

In these cases, you might have trouble using var (I say "might" because it is more like "undefined behavior") because FooNodeCollection is both IEnumerable<Node> and IEnumerable<FooNode> (if I understand correctly). So, in that context, whenever you type var, you might have the problem you mention, i.e. the runtime will not know what to cast your object to.

However, the use of inherited (rather than adjacent) hierarchy types gives you the option to resolve the inconsistency using OfType<T>. If the runtime gets the enumerator of the base type, your objects are certainly of the base type, so they will all be enumerated. If the enumerator of the derived type runs, of course your objects have been constrained (by the constructor), so your are still safe and they will definitely be of that specific type, hence all will be enumerated.

In short, if you are casting to the correct type with OfType<T>, you should not have a problem, plus the var will be of the cast type T, so you will have compile-time analysis. However, because I think you are probably OK with the visitor and you only take into account what to do with a type only at the single-node level, you will probably not run into this type of problem, as you will be operating within the "collection-type-agnostic" VisitCollection scope. In that case, though, I don't see why you would want to make the inheriting classes implement IEnumerable<DerivedType> as well.