1

I have too many downcasts in my code. In c++ I can use templates to avoid downcasting. But what is the best implementation of the following example in c#?

class Pet { bool mIsDog; }
class Dog : Pet // mIsDog = true
class Cat : Pet // mIsDog = false

class Owner //There are many different owner classes, may come from different dlls
{
    private List<Food> mFoods;

    private void FeedDog(Dog dog)
    {
        ... //Use mFoods, which is invisible to dog
    }

    void HandlePet(Pet pet)
    {
        if(pet.mIsDog) //In c++ I can use templates to avoid downcasting
            FeedDog((Dog)pet);
        else
            FeedCat((Cat)pet);

        ... //code handling general pet

        while(pet.IsUnhappy())
        {
            if(pet.mIsDog) //In c++ I can use templates to avoid downcasting
                PlayWithDog((Dog)pet);
            else
                PlayWithCat((Cat)pet);
        }

        ... //a lot of code handling general pet or specific pet (cat or dog)
    }
}

Note that function HandlePet has very complex logic with many levels of indentations so it is difficult to break it into multiple seperate functions.


The reason that I do not make BeFedWith or BePlayedWith a virtual function within Pet class is that I may have many different Owner classes, e.g., BoyOwner, GirlOwner, WomanOwner, ManOwner, each with its own way to feed the pet. Pet is a common class and is used by many other classes, but Owner classes only interact with Pet. Also, functions such as FeedDog need to access the private members of Owner class.

Fan
  • 315
  • 3
  • 11
  • 6
    Is there any reason that `Pet` isn't an abstract class with `Play` and `Feed` methods? – Jon Skeet Apr 24 '13 at 15:06
  • You could add an abstract `BeFed` (or similarly named) method to your Pet class and call that from your owner class then you don't need to get the concrete class, just the (now abstract) `Pet` class is enough. (edit: what he said!) – George Duckett Apr 24 '13 at 15:07
  • 4
    Having `mIsDog` in `Pet` is a very, very bad design decision. – Daniel Hilgarth Apr 24 '13 at 15:08
  • Feed and PlayWith need to use private members of class Owner. Also Owner is like a seperate module which may or may not be compiled so I would like to keep most stuff within itself. Cat and Dog are big classes already. – Fan Apr 24 '13 at 15:11
  • 3
    @user2316040: Then you have other serious design issues. I'm pretty sure a Pet and an Owner messing with each other's privates that much, constitutes animal abuse. :P – cHao Apr 24 '13 at 15:19
  • See the edit to my answer. – George Duckett Apr 24 '13 at 15:24

3 Answers3

1

The problem you are try to solve is how to dispatch based on object type, when you don't know at the time you design the object all the different operations that might be required. (The other answers suggest putting all the required operations in the base class; this is sometimes not possible and leads to excessive coupling).

The answer is not to test the type of the object and cast; you should be doing that seldom, if ever. A good solution is to use the Visitor pattern: when you make the object Visitable, you can add new operations dispatched on the object type by implementing the operation, not by changing the object itself.

antlersoft
  • 14,636
  • 4
  • 35
  • 55
0

Well, downcasting is considered bad design... you should move common method to baseclass and make them abstract or have an interface to do that.

if you have:

public interface Pet{
    void Feed();
    void Play();
}

public class Dog:Pet{
    public void Feed(){
        ...dog feeding code...
    }
    public void Play(){
        ...dog playing code...
    }
}
public class Cat:Pet{
    public void Feed(){
        ...cat feeding code...
    }
    public void Play(){
        ...cat playing code...
    }
}

then you handlePet function become:

void HandlePet(Pet pet){
    pet.Feed();
    while(pet.isUnhappy())
        pet.Play();
}
Fabio Marcolini
  • 2,315
  • 2
  • 24
  • 30
  • Feed and PlayWith need to use private members of class Owner. Also Owner is like a seperate module which may or may not be compiled so I would like to keep most stuff (Feed, Play) within itself. – Fan Apr 24 '13 at 15:14
  • 1
    to avoid using a downcast you may go and search for dynamic keyword in c#, but still is a pretty bad design choice and I'm not sure if dynamic is the right way, anyway another thing instead of using variable mIsDog you can use if(pet is Dog) to check if pet is a class implementing Dog – Fabio Marcolini Apr 24 '13 at 15:24
  • @FabioMarcolini: See my example for using `dynamic`. Still agree it's not ideal. – George Duckett Apr 24 '13 at 15:29
  • @user2316040: from what I see you're not very eager to refactor your design so I think best solution as i suggested, look at George Duckett answer for usage. Anyway be aware that dynamic is not typesafe and your design will become very hard for you to implement code as your project go on and as it is is not flexible (its very hard to implement a new type of pet) – Fabio Marcolini Apr 24 '13 at 15:36
  • I am happy to refactor the design if that is feasible. I have updated my question to show why it is difficult to put "BeFed" in Pet class. – Fan Apr 24 '13 at 15:47
  • @user2316040: I've updated my answer with an example using dynamic, as well as the visitor pattern. – George Duckett Apr 24 '13 at 16:00
0

If you must do what you suggest, keeping the Feed etc. methods part of the owner not the pet, then you could utilize dynamic binding to call the correct owner method:

using System;

class Pet { }
class Dog : Pet { }
class Cat : Pet { }

class Owner
{
    public void HandlePet(Pet pet)
    {
        HandlePet((dynamic)pet);
    }

    private void HandlePet(Cat pet)
    {
        Console.WriteLine("Stroke the cat softly whispering Please don't scratch me.");
    }

    private void HandlePet(Dog pet)
    {
        Console.WriteLine("Stroke the dog firmly saying Who's a good boy?");
    }
}

class Program
{
    static void Main(string[] args)
    {
        var owner = new Owner();

        Console.WriteLine("Handle dog first");
        owner.HandlePet(new Dog());

        Console.WriteLine("Handle cat second");
        owner.HandlePet(new Cat());

        Console.ReadKey();
    }
}

Output:

Handle dog first
Stroke the dog firmly saying Who's a good boy?
Handle cat second
Stroke the cat softly whispering Please don't scratch me.


If you can avoid it, then I would put the methods on the Pet classes, as below:
One way would be to make Pet an abstract class, which has EatFood and Play abstract methods. The Dog and Cat classes would implement these methods:

using System;

abstract class Pet
{
    public abstract void EatFood();
    public abstract void Play();

    public void BrushHair()
    {
        Console.WriteLine("Hair brushed!");
    }
}
class Dog : Pet
{
    public override void EatFood()
    {
        Console.WriteLine("Eating dog food...");
    }

    public override void Play()
    {
        Console.WriteLine("Run around manically barking at everything.");
    }
}
class Cat : Pet
{
    public override void EatFood()
    {
        Console.WriteLine("Eating cat food...");
    }

    public override void Play()
    {
        Console.WriteLine("Randomly choose something breakable to knock over.");
    }
}

class Owner
{
    public void HandlePet(Pet pet)
    {
        pet.EatFood();
        pet.Play();
        pet.BrushHair();
    }
}

class Program
{
    static void Main(string[] args)
    {
        var owner = new Owner();

        Console.WriteLine("Handle dog first");
        owner.HandlePet(new Dog());

        Console.WriteLine("Handle cat second");
        owner.HandlePet(new Cat());

        Console.ReadKey();
    }
}

Output:

Handle dog first
Eating dog food...
Run around manically barking at everything.
Hair Brushed!
Handle cat second
Eating cat food...
Randomly choose something breakable to knock over.
Hair Brushed!


If you want to use the visitor pattern, try the code below:

using System;

public interface IPetVisitor
{
    void Visit(Dog dog);
    void Visit(Cat cat);
}

public interface IPetAcceptor<T>
{
    void Accept(T visitor);
}

public abstract class Pet : IPetAcceptor<IPetVisitor>
{
    public abstract void Accept(IPetVisitor visitor);
}
public class Dog : Pet
{
    public override void Accept(IPetVisitor visitor)
    {
        visitor.Visit(this);
    }
}
public class Cat : Pet
{
    public override void Accept(IPetVisitor visitor)
    {
        visitor.Visit(this);
    }
}

class Owner
{
    // Private variable on owner class
    private int HandCount = 2;

    // Pet handler is an inner class so we can access the enclosing class' private member.
    public class PetHandler : IPetVisitor
    {
        private Owner Owner;

        public PetHandler(Owner owner)
        { Owner = owner; }

        public void Visit(Dog dog)
        {
            Console.WriteLine("Pet the dog with {0} hands", Owner.HandCount);
        }

        public void Visit(Cat cat)
        {
            Console.WriteLine("Pet the cat with {0} hands", Owner.HandCount);
        }
    }
    private PetHandler PetHandlerInstance;

    public Owner()
    {
        PetHandlerInstance = new PetHandler(this);
    }

    public void HandlePet(IPetAcceptor<IPetVisitor> pet)
    {
        pet.Accept(PetHandlerInstance);
    }
}

class Program
{
    static void Main(string[] args)
    {
        var owner = new Owner();

        Console.WriteLine("Handle dog first");
        owner.HandlePet(new Dog());

        Console.WriteLine("Handle cat second");
        owner.HandlePet(new Cat());

        Console.ReadKey();
    }
}
George Duckett
  • 31,770
  • 9
  • 95
  • 162
  • 'dynamic' seems to be what I want. It works in a similar way as c++'s template. But why is it a bad design? – Fan Apr 24 '13 at 15:34
  • 1
    What happens for example, if you forget to have a HandlePet method that accepts a dog, or add a third type of pet? – George Duckett Apr 24 '13 at 15:35
  • It gives me a runtime error. Too bad it cannot check it during compiling like c++ does. – Fan Apr 24 '13 at 15:41
  • that's the main problem of dynamic, it's not typesafe – Fabio Marcolini Apr 24 '13 at 15:42
  • @user2316040: Edited in an example of the visitor pattern. – George Duckett Apr 24 '13 at 15:47
  • @user2316040: When you use `dynamic` generally you lose compile time checking in return for being able to use it as you can. For compile time checking maybe look into one of the alternative designs I suggest. – George Duckett Apr 24 '13 at 15:49
  • It seems the vistor pattern is more robust but a little cumbersome because I need to create a class for every action (feed, play). Is it a good practice if I have many such actions? – Fan Apr 24 '13 at 16:02
  • You could make `Owner` implement `IPetVisitor` directly (rename it to `IPetFeeder` for example) and make the methods `Handle` or something. Then you wouldn't have the extra class, you'd make multiple interfaces, like `IPetFeeder`, `IPetPlayer` etc. and the `Owner` would have `Feed` and `Play` methods for each pet type. – George Duckett Apr 24 '13 at 19:24