Is this design using dynamic okay?

Question

I've got a list of different jobs to process in my application. I'm toying with a design which uses different types to represent different types of job - this is natural because they have different properties and so on. For processing I was thinking about using the dynamic keyword in C# as per the code below.

abstract class Animal {}    
class Cat : Animal {} 
class Dog : Animal {}

class AnimalProcessor
{
    public void Process(Cat cat)
    {
        System.Diagnostics.Debug.WriteLine("Do Cat thing");
    }

    public void Process(Dog dog)
    {
        System.Diagnostics.Debug.WriteLine("Do Dog thing");
    }

    public void Process(Animal animal)
    {
        throw new NotSupportedException(String.Format("'{0}' is type '{1}' which isn't supported.",
            animal,
            animal.GetType()));
    }
}

internal class Program
{
    private static void Main(string[] args)
    {
        List<Animal> animals = new List<Animal>
        {
            new Cat(),
            new Cat(),
            new Dog(),
            new Cat()
        };

        AnimalProcessor animalProcessor = new AnimalProcessor();
        foreach (dynamic animal in animals)
        {
            animalProcessor.Process(animal);
        }

        //Do stuff all Animals need.
    }
}

The code works as expected, but, I have a nagging feeling that I'm missing something blindingly obvious and that there's a much better (or better known) pattern to do this.

Is there any better or equally good but more accepted pattern to use to process my Animals? Or, is this fine? And, please explain why any alternatives are better.

Answer 1

You are not missing anything: dispatching on runtime type of the object is something that dynamic can do quite well.

You do have alternatives, such as implementing the visitor pattern, but it is much costlier in terms of implementation, and it is also not nearly as readable as your approach that uses dynamic:

interface IAnimalProcessor { // The visitor interface
    void Process(Cat cat);
    void Process(Dog dog);
    void Process(Animal animal);
}
class AnimalProcessor : IAnimalProcessor {
    ...
}
interface IProcessable {
    void Accept(IAnimalProcessor proc);
}
class Cat : IProcessable {
    public void Accept(IAnimalProcessor proc) {
        proc.Process(this); // Calls the Cat overload
    }
}
class Dog : IProcessable {
    public void Accept(IAnimalProcessor proc) {
        proc.Process(this); // Calls the Dog overload
    }
}
...
AnimalProcessor animalProcessor = new AnimalProcessor();
foreach (IProcessable animal in animals) {
    animal.Accept(animalProcessor); // The animal will call back the right method of the proc
}

Answer 2

This is definitely bad design.
You simply can't introduce interface for AnimalProcessor, as adding new Animal subclass will cause interface to change.
More over, the client of your AnimalProcessor should know about the special construct(foreach with dynamic) for class usage. If you still want to use "dynamics" in this case, consider something like:

class AnimalProcessor
{
    public void Process(Animal animal)
    {
        dynamic concreteAnimal = animal;

        DoProcess(concreteAnimal);
    }

    private void DoProcess(Animal animal)
    {
        throw new NotImplementedException();
    }

    private void DoProcess(Cat cat)
    {
        System.Diagnostics.Debug.WriteLine("Do Cat thing");
    }

    private void DoProcess(Dog dog)
    {
        System.Diagnostics.Debug.WriteLine("Do Dog thing");
    }
}

At least, you will be able to extract interface in future and have different implementations of AnimalProcessor. And still you will have one serious problem: this is absolutely not obvious, that after adding subclass you need to add method to some standalone class, or your system will throw.