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) {
...
}
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();
}
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With