TL;DR
You're violating the Open/Closed Principle by writing your code in a way that requires constant updating when classes are added to the assembly. You're also violating LSP by expecting that a base class can do more than its definition defines.
Note: The link refers to LSP, but the top answer show a workaround which in turn violates OCP, and that is the same as what you're trying to do in your case.
The example bad code in the linked answer:
void MakeDuckSwim(IDuck duck)
{
if (duck is ElectricDuck)
((ElectricDuck)duck).TurnOn();
duck.Swim();
}
is the same as what you're describing:
void MakeCardDoSomething(C c)
{
if (c is IBlockingC)
((IBlockingC)c).Block();
c.DoSomething();
}
The rest of the answer explores your question and expectations of how things should work. But any answer that addresses OCP will also answer your question. I'm just trying to put it closer to home for you.
Wrong expectations
C1 supports blocking and unblocking of cards. C2 does not support that but the rest of the features are the all same.
So, I have implemented an abstract class, which holds the common denominator C. C1and C2 extends C.
Just to elaborate: this means that C does not contain any blocking logic. Someone who handles a C type object has no reasonable expectation to use blocking features.
Now say some malicious user tries blocking a card which does not support the block card feature. I think I can not leverage my CardFactory because the return type of CardFactory is C and that does not satisfy the type expected by code calling .blockCard. So either I need to change the placeholder to C and check if the method exists or not. Or I can put the logic in the constructor only to check if the card supports the feature and then throw an error if not.
If your method's return type is C, then the caller cannot expect the ability to block cards. It would be counterintuitive for the caller to expect something they are not receiving.
When you say "Foo() returns C", you state that it returns any valid implementation of C. Since the class definition of C does not contain blocking logic, then you cannot guarantee that every valid implementation of C has blocking logic, and therefore you cannot acknowledge its existence.
In reality, it's possibly to cast C to C1 and then call the blocking logic, but your cast is a guess. And it's not what you're supposed to do. It's not logically consistent, and it's a dirty patch to a glaring problem of polymorphism abuse.
In short, a common interface (whether it's an interface, abstract base class or concrete base class doesn't matter) should be handled as a black box with buttons. When you (as the consumer) are dealing with a common interface, you do not know what's happening under the hood, but you know how to operate the black box (because it has all the necessary buttons).
A real world example
Think of it like a vending machine. You put a coin in the slot, you press a button, the drink comes out. As the customer (consumer), you only care about three things:
- I can put a coin in.
- I can press a button.
- When I put a coin in and press the button, I get a drink.
This is the interface. It defines what exchange takes place between you and the machine. This does not define how the machine works. There could be many different types of vending machines:
- An electrical mechanical device like we have in real life.
- There is a person in there who will fetch the right drink and put it through the slot.
- There are hamsters in there who are fed by coins and give drinks.
- Inside the machine there is a portal to another dimension, where coins get transmuted to soda cans and then sent back through the portal.
It simply doesn't matter. As long as you get the drink you wanted, the transaction between you and the vending machine is satisfied.
Hiding the innards of the black box
When you want variations on basic behavior, i.e. C1 and C2 behave differently but both can be handled like a C, what you need to do is hide their differences. Every derivation of C needs to appear to behave the exact same way. Their actual behavior under the hood will be different (because otherwise you wouldn't need different classes), but a consumer can interact with all of them the exact same way.
For example:
public abstract class C
{
public abstract int CalculateDamageOutput(int damage);
}
We've defined a C, and we've stated that every valid implementation of C is able to calculate damage output. We have not defined how damage output is calculated, we have only defined that it can be calculated.
And now we start implementing it. Please note that I am skipping the additional interfaces as they are not relevant to the answer. The subclasses can also implement additional interfaces, and the principle remains the same.
public class WeakC : C // Takes double damage
{
public override int CalculateDamageOutput(int damage)
{
return damage * 2;
}
}
public class BlockingC : C // Can mitigate a certain amount of damage.
{
private bool BlockedDamageLastTime = false;
private int ArmorValue = 15;
public override int CalculateDamageOutput(int damage)
{
if(this.BlockedDamageLastTime)
{
// You can't block twice in a row
this.BlockedDamageLastTime = false;
return damage;
}
var output = Math.Max(0, damage - this.ArmorValue);
// Armor loses efficiency every time you block
if(this.ArmorValue > 0)
this.ArmorValue--;
// Prevents blocking the next time.
this.BlockedDamageLastTime = true;
return output;
}
}
public class InvincibleC : C // Does not take damage
{
public override int CalculateDamageOutput(int damage)
{
return 0;
}
}
These are just some examples of very different behaviors. Some are more complex than others. But the important thing to consider here is what does this mean for a consumer?
var originalAttackDamage = 25;
var currentC = CFactory.Get();
var actualAttackValue = currentC.CalculateDamageOutput(originalAttackDamage);
Try to figure out which subclass of C is being used here. Can you tell me which one we're using? No, you can't, because it doesn't matter. This code works for any valid implementation of C.
Even if you end up developing new subclasses of C and change the CFactory.Get() return value to return an object of type MyNewlyDevelopedC, this code does not need to be updated or rechecked, as the logic remains the same.
Direct answers
- So, I have created a factory for it. It takes the data and returns proper concrete object depending on the data set it is getting. Is it the right approach or is it ok to put the conditionals in client code for proper object creation?
It is a correct approach, but you need to take care of the return type that your factory method has.
If your factory method returns a C, then the consuming code cannot expect to receive an object that has capabilities that are not defined in C (or any of the classes it derives from).
If your consumer is expecting to receive an object which is capable of blocking, then your method should have the IBlockingC return type, not the C return type.
Now say some malicious user tries blocking a card which does not support the block card feature. I think I can not leverage my CardFactory because the return type of CardFactory is C.
Regardless of the user being malicious, it makes no sense for a user to ask for a particular object (a blocking C) from a method whose signature does not stipulate that it returns blocking C objects.
Or I can put the logic in the constructor only to check if the card supports the feature and then throw an error if not.
By "the card", I assume you mean the C class. Your C class should NEVER EVER be aware of its own subclasses. The purpose of the C class is to provide something that you expect all subclasses to share. Its purpose is not to act as a collection of references to all its subclasses.
The subclass references the base class. The base class never references the subclass.
If I remove the factory then every time a new card is created I need to do an if-else check. What's the best way to achieve it?
You're putting the cart before the horse. If you want to work under the "block if you can, don't if you can't" principle, then you shouldn't be using subclasses here.
What you have done is created subclasses which innately (and definitively) define whether or not this object is capable of blocking. That means that when you create the correct object, you are already deciding whether the ability to block is needed or not.
Later, when you pass that object around, you will therefore already be aware whether you created a blocking object or not. If you are not aware of it anymore, there are two possibilities:
- If you (the consumer) had no say in it and don't know what happened under the hood, that means that you (the consumer) are not supposed to know if this object can block or not.
- If you (the consumer) had a say in it, but you simply don't remember anymore, that means you're mismanaging your data and have simply lost some information that was important to you. This is not solved by reobserving the object. This is solved by making sure you don't lose that contextual information in the first place.
In either case, the solution is not to look at the object and ask yourself "can this block? Because I don't know!". Either you're not supposed to know, or you already know.
