Using the visitor pattern with generics in C#

Question

I want to know whether the below is an acceptable use of the visitor pattern. I feel a little uncomfortable returning from an Accept() or Visit() call - is this an appropriate usage of this pattern and if not, why not?

Note: Apologies for the long code sample, seems necessary to get across what I'm doing as visitor always seems to be a little involved...

interface IAnimalElement<T>
{
   T Accept(IAnimalVisitor<T> visitor);
}

interface IAnimalVisitor<T>
{
    T Visit(Lion lion);
    T Visit(Peacock peacock);
    T VisitZoo(List<Animal> animals);
}

abstract class Animal
{
    public int Age { get; protected set; }
}

class Lion : Animal, IAnimalElement<int>
{
    public Lion(int age)
    {
        Age = age;
    }

    public int Accept(IAnimalVisitor<int> visitor)
    {
        return visitor.Visit(this);
    }
}

class Peacock : Animal, IAnimalElement<int>
{
    public Peacock(int age)
    {
        Age = age;
    }

    public int Accept(IAnimalVisitor<int> visitor)
    {
        return visitor.Visit(this);
    }
}

class AnimalAgeVisitor : IAnimalVisitor<int>
{
    public int TotalAge { get; private set; }

    int IAnimalVisitor<int>.Visit(Lion lion)
    {
        TotalAge += lion.Age;
        return lion.Age;
    }

    int IAnimalVisitor<int>.Visit(Peacock peacock)
    {
        TotalAge += peacock.Age + 10;
        return peacock.Age + 10; // peacocks ages are always -10y, correct.
    }

    public int VisitZoo(List<Animal> animals)
    {
        // Calculate average animal age.

        int sum = 0;
        int count = 0;
        foreach (IAnimalElement<int> animal in animals)
        {
            sum += animal.Accept(this);
            ++count;
        }

        return count == 0 ? 0 : sum / count;
    }
}

class Program
{
    static void Main(string[] args)
    {
        List<Animal> animals = new List<Animal>() { new Lion(10), 
          new Lion(15), new Peacock(3), new Lion(2), new Peacock(9) };

        AnimalAgeVisitor visitor = new AnimalAgeVisitor();

        Console.WriteLine("Average age = {0}, Total age = {1}", 
            visitor.VisitZoo(animals), visitor.TotalAge);
    }
}

Hyperbolic

Oh btw you should totally use the [DebuggerStepThrough] attribute on your Accept methods. – Shaylashaylah

Answer 1

Well to me this feels like the implementation is a little bit on the fence.

Either have your Visit and Accept methods return void and track all the state in the Visitor object. Interrogate it at the end.

or ...

Have Visit and Accept return an in-progress state and accept an incoming in-progress state in a functional manner.

If you go for the second option I'm not really sure that a visitor object or pattern is needed, you can use an iterator, function and some transient state instead.

Offshore

Answer 2

Short answer: I don't see any problems of exposing a IVisitor returning a generic parameter.
See FxCop rules.

It then permits to use different IVisitor each returning a different value.

However, in your case, Visitor is not useful, since every animal has the Age property so all can be done with Animal or a new IAnimal interface.

Alternative is using multiple-dispatch at the cost of losing Strong Typing.

Use a Visitor pattern when you want to replace (or avoid to write) a switch like this one:

IAnimal animal = ...;
switch (animal.GetType().Name)
{
  case "Peacock":
    var peacock = animal as Peacock;
    // Do something using the specific methods/properties of Peacock
    break;
  case "Lion":
    var peacock = animal as Lion;
    // Do something using the specific methods/properties of Lion
    break;
   etc...
}

or the nested if-then-else equivalent.

It's purpose is to route the instance to the routine relevant for its type by using polymorphism and then avoid ugly if-then-else/switch statements and manual casts. Furthermore, it helps to decrease coupling between unrelated code.

Alternative to that is to add a virtual method in the class tree to visit. However, sometimes it's not possible or desirable :

• visitable class code not modifiable (not owned for example)
• visitable class code not related to visiting code (adding it in class would mean lowering the cohesion of the class).

That's why it's often used to traverse an object tree (html nodes, lexer tokens, etc...). Visitor pattern implies the following interfaces:

• IVisitor

  /// <summary>
  /// Interface to implement for classes visiting others. 
  /// See Visitor design pattern for more details.
  /// </summary>
  /// <typeparam name="TVisited">The type of the visited.</typeparam>
  /// <typeparam name="TResult">The type of the result.</typeparam>
  public interface IVisitor<TVisited, TResult> : IVisitor where TVisited : IVisitable
  {
      TResult Visit(TVisited visited);
  }
  
  /// <summary>
  /// Marking interface.
  /// </summary>
  public interface IVisitor{}
  

• IVisitable

  /// <summary>
  /// Interface to implement for classes visitable by a visitor.
  /// See Visitor design pattern for more details.
  /// </summary>
  /// <typeparam name="TVisitor">The type of the visitor.</typeparam>
  /// <typeparam name="TResult">The type of the result.</typeparam>
  public interface IVisitable<TVisitor, TResult> : IVisitable where TVisitor : IVisitor
  {
      TResult Accept(TVisitor visitor);
  }
  
  /// <summary>
  /// Marking interface.
  /// </summary>
  public interface IVisitable {}
  

Implementation of Accept in each IVisitable should call Visit(this).

Ambert

Answer 3

The visitable accept method is not supposed to return anything. The accept is only supposed to indicate the visitor what to visit after or during the visit.

Dunstan

Answer 4

It's fairly common. I don't know if you can do it in C#, but in Java it's normal to leave the Accept method generic, so what's returned is decided by the visitor not the visitee:

interface IAnimalElement
{
   <T> T Accept(IAnimalVisitor<T> visitor);
}


interface IAnimalVisitor<T> 
{
   T Visit(Peacock animal);
  ...
}

For procedures, a IAnimalVisitor<Void> returning null can be used.

Purusha