Concrete type or Interface?

Question

I have the following use case , lot of code which was tightly coupled on a concrete type (say Concrete1). Later figured out the concrete type needs to be changed, so defined an interface . E.g

Class ABC {

 virtual int foo() = 0;
 virtual int getType() = 0;

}

class Concrete1 : public ABC {

    int foo() {
    ... }
    int getType() {
      return 1;
    }  

} 
class Concrete2 : public ABC {
    int foo() {
    ... }
    int getType() {
      return 2;
    }
 }

A static factory pattern was used for creation of the objects. So all places where the object new Concrete1 was created is replaced with ABCFactory::createType().

Now there are a lot of places in code where I need to check if the object returned by createType is whether Concrete1 or Concrete2 and accordingly do the relevant logic (So a lot of if else in the code :( ).

I want to avoid a lot of if else in the code as part of this change. Any suggestions?

The thing which bothers me a lot is

if (abc.getType() == 1) {
    ...
} else if (abc.getType() ==2) {
    ...
}

Answer 1

Can you move the places that you are detecting object types outside of the class into the class? That way the functionality (that apparently depends upon the specific class) is actually associated with the appropriate class?

Answer 2

Put the common functionality into your base class, and use polymorphism instead of conditional/switch statements.

Answer 3

the entire point of using interfaces is so that you can use polymorphism which means you should never have to check what type an instance is. doing so is a very big code smell (see Fowlers Refacotring). move the conditional logic to the concrete classes and add te function that will handle it to the interface

EDIT (Adding code example since initial post was done from cell phone):

You are trying to do:

void Main(string[] args)
{
   Bird bird = BirdFactory.GetPigeon();
   if (bird.GetType().Equals(typeof(Duck)))
   {
      Console.WriteLine("quack");
   }
   else if (bird.GetType().Equals(typeof(Pigeon)))
   {
      Console.WriteLine("coo coo");
   }
}

Instead, try:

interface Bird
{
    void Speak();
}

class Duck : Bird
{
    void Speak()
    {
        Console.Write("quack");
    }
}

class Pigeon : Bird
{
    void Speak()
    {
        Console.Write("coo coo");
    }
}

void Main(string[] args)
{
    Bird bird = BirdFactory.GetPigeon();
    bird.Speak();
}

Answer 4

If you're checking for and/or switching on type at runtime, you may want to consider using Run Time Type Information, if it's available for your compiler. It does add some overhead, but it accomplishes what you're trying to do without creating or maintaining a custom methodology.

Unlike purists, I'm not really a "put the functionality in the class so you can use polymorphism to get around the type switching" kinda person. Both are valid approaches, though (although the "in the class" method is not always possible, and/or conceptually clean).

Answer 5

By way of agreement with the other answers, it sounds to me like at least some of the code in the if/else blocks needs to be moved inside the concrete classes as a new virtual function. That would allow you to exploit polymorphism rather than switch on types with a homebrewed reflection pattern.

Answer 6

Put the ... inside the implementation of yet another virtual method:

if (abc.getType() == 1) {
    ... // A
} else if (abc.getType() == 2) {
    ... // B
}

Put A and B like this:

class ABC {
 virtual int foo() = 0;
 virtual void doIt() = 0; // choose a proper name
};

class Concrete1 : public ABC {
    int foo() {
    ... }
    void doIt() {
    ... // A
    }
};

class Concrete2 : public ABC {
    int foo() {
    ... }
    void doIt() {
    ... // B
    }
 };

And change your if to

abc.doIt();

As another one said, that's exactly the point of dynamic dispatch! Beside being more terse, it also will never "forget" to handle a type. Doing your switch, you could silently not handle a particular type, because you missed updating the code at that place when you introduced a new implementation. Also remember having a virtual destructor in ABC.