0

It’s frequently taught that downcasting is bad practice. I have a case where it seems necessary, but I’m wondering if there is an alternative way to design this. Suppose interfaces are kept small through interface inheritance. Let’s use this example (source): enter image description here

An advantage of interfaces is that a ParkingLotManager can store a list of ParkingLot objects, which can be FreeParkingLot, PaidParkingLot, etc.

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? If it’s still considered bad practice, what could be a better design that avoids this?

3
  • 1
    What kinds of operations are you wanting to do on the objects?
    – mmathis
    Aug 25, 2021 at 2:28
  • @mmathis Simplest operation would be just logging the free parking lots. But ideally the design would be compatible with clients having other use cases. Ex: A client that fetches from ParkingLotManager and calculates the expected fee for paid lots.
    – justin
    Aug 25, 2021 at 2:32
  • 1
    By the way, the diagram represents only navigable associations and no inheritance. This is confusing. You need to use larger plain white triangles as arrow-head to tell that it’s inheritance.(at least in UML)
    – Christophe
    Aug 25, 2021 at 7:10

4 Answers 4

1
  1. It’s frequently taught that downcasting is bad practice.
  2. 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.

2

Casting is a bad practice. Not just because it is potentially unsafe, but because it indicates a failed abstraction or some design issue.

In this case, you really shouldn't want to "check for capabilities" on parking lots. What you should do instead is to ask the parking lot to perform its duties however it can. You can ask it to park() a car for example and then the ParkingLot itself will decide whether to bill the car or not, with how much, etc.

In other words, your abstractions work, if the details can remain hidden. If not (for example you need to cast), your abstraction simply doesn't work.

2
  • Surely the caller might want to know if the parking lot is free before attempting to park? And if it's not free, what payments are acceptable, and how much the cost is, and if it's free on weekends? Where do you think we should put properties for those? I understand you are thinking about "tell don't ask", but sometimes the caller really does want to ask.
    – John Wu
    Aug 26, 2021 at 19:32
  • If we are talking about a single application, no, you really don't want to ask. Asking for stuff in code, even though you had the whole suite of requirements beforehand, is a tacit admission that your abstractions don't work, or you couldn't think of any. For example: If the human user wants to decide based on those things, that is a presentation issue. You don't have to expose all those properties just for the user, expose a single presentation behavior instead. Aug 27, 2021 at 6:45
1

Starting out with the original question

Well, casts are there for a reason, so they cannot be inherently bad.

BUT: if you use them in the way you describe, i.e. your ParkingLotManager can only return the list of all parking lots, and then you have to filter on the client side, this is smelly. Normally, you'd have your parking lots organized in some kind of backing storage where you can select the parking lots according to some given criteria. (A database comes to mind.) So, your ParkingLotManager should probably have some specialized query methods, which return the specialized objects in the first place.

Therefore, if you need the list of FreeParkingLot instances, do not query List<ParkingLot> ParkingLotManager.getAllParkingLots() and then filter on the client, but call List<FreeParkingLot> ParkingLotManager.getFreeParkingLots() which will already give you the correctly typed list.

This also has the advantage of sending less objects through your application.

Going a step back

Specializing classes with additional and potentially limiting attributes is mostly in violation of the Liskov substitution principle. This comes from the (mostly wrong) idea, that the relation X is a Y is often wrongly modeled via inheritance.

Your parking lot is probably inheritly a piece of land. It is not inheritly paid or free, as well as it is not statically and permanently taken or free. Rather, your parking lot has an attribute which constitutes something like a payment plan. This payment plan can change over the lifetime of this piece of land without any change to the actual parking lot.

The advice I usually give at this place is reading the Wizards and Warriors essay by Eric Lippert, which covers the topic much better than a mere answer on stackexchange can do. (Unless this answer is given by Flater, maybe you'll get lucky ;-))

1

In general, in a polymorphic design:

  • Prefer to tell and not ask: instead of looking at each object and doing something depending on its class or abilities, tell the object what you want and let the object handle the operation (or a part of it, see template method pattern).
  • But this is not desirable if it would require to add unrelated responsibilities to a class and thus break sound separation of concerns or SRP (e.g in your case doing statistics and reporting on parking lots seems a distinct responsibility). In this case, you should consider decoupling. The visitor pattern could help, but only if the classes are not in a deep and very dynamic hierarchy.
  • Only in last ressort should you think of using casting or introspection. Last, because it creates the kind of strong coupling that the principle of least knowledge tries to avoid.

In your illustrative case, and whenever a class hierarchy goes to deep:

  • Take a deep breath and wonder if you should prefer composition over inheritance.
  • The constant or hourly fee is a commercial strategy, which could probably change over time for the same lot even after its creation. The strategy pattern is an example of composition over inheritance that seems well suited for this kind of needs.
  • The extreme approach in this regard is the entity component system design, which is popular in the gaming industry: The class hierarchy is ultra-flat and the composition of behaviors gives the possibility of almost endless combinations (e.g. a paid parking lot hourly the day, constant over night with an automatic cleaning service for a small additional fee ;-) )

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.