- It’s frequently taught that downcasting is bad practice.
- An advantage of interfaces is that a ParkingLotManager can store a list of ParkingLot objects, which can be FreeParkingLot, PaidParkingLot, etc.
People doing #2 is precisely the reason why #1 is taught.
What you're doing here is abusing polymorphism so you can cut a corner and store lots of different things in the same location. This isn't being done because you're relying on polymorphic implementation enabling you to treat these objects via their base type. It's being done to not have to bother with storing things in more than one place.
While that may be nice from a storage perspective, it effectively negates your typing, which is the main benefit (and intended approach) of the language you're using.
If a client wants to fetch parking lots from this ParkingLotManager, and perform operations on a FreeParkingLot, is it acceptable to check the ParkingLot capabilities by attempting to downcast to a FreeParkingLot?
"Acceptabble" is the wrong word to use here, because that's inherently subjective. Is it acceptable that you e.g. use single letter global variables? Most developers would say no, but there's probably still some core software in the world running this way and working reliably and the company has no interest in reworking it for good practice's sake alone. In that sense, it would also be "acceptable", even if most (if not all) developers disagree.
If it’s still considered bad practice, what could be a better design that avoids this?
The main point of strong typing revolves around knowing what you're handling. The situation you've created is one where you don't know what you're handling, and you have to dynamically figure it out.
When you cast something to a more base type, what you're effectively saying is "I no longer care about those specific details and will only treat you as a more generalized thing". There's no coming back from that without having to brute-force check for the specific type again, which is inelegant and an OCP violation waiting to happen.
While that is not impossible to implement from a technical point of view, it is a really bad design, leading to constant null and/or canDoX checks, which in turn start causing all kinds of LSP violations or shoddy workarounds that do the bare minimum to not count as an LSP violation.
The main issue here is an overreliance on inheritance, and subsequently an overspreading of the possibilities in different branches of this inheritance graph.
For example, I'd argue that FreeParkingLot has no purpose, as this can simply be defined as a PaidParkingLot whose price always returns 0. This innately solves the problem at hand (for those two types), since you've just reduced the ParkingLot/FreeParkingLot/PaidParkingLot love triangle back down to a single type, which I'd say should be ParkingLot then (whose implementation is precisely that of the original ParkingLot + PaidParkingLot.
As to the hourly/constant fee distinction, you've been tricked by semantical English. You've used inheritance because you thought of it in terms of "X is a constant fee parking lot", but it's more appropriate to think of it as "X is a parking lot whose price is a constant fee". In other words, the price calculation should favor composition over inheritance.
public class ParkingLot
{
private readonly IFeeCalculator _feeCalculator;
public ParkingLot(IFeeCalculator feeCalculator)
{
_feeCalculator = feeCalculator;
}
public string PrintFee(CarDetails details)
{
var fee = _feeCalculator.Calculate(details);
return fee > 0
? $"${fee}"
: "Free";
}
}
You can implement IFeeCalculator classes as you see fit.
- A
NoFeeCalculator which always returns 0
- A
ConstantFeeCalculator which returns a preset amount
- A
CarTypeFeeCalculator which calculates a price based on vehicle type and duration of stay
- ...
And if you want, you can still inherit your ParkingLot to hardcode certain behaviors related to specific fee calculators:
public class FreeParkingLot : ParkingLot
{
public FreeParkingLot() : base(new NoFeeCalculator()) {}
}
But this is only useful if there is something meaningful to add to this class.
