Extracting interface or use double dispatch to avoid downcasting

designinterfacesobject-oriented

Here's an analogy of our concrete problem to demonstrate the issue at hand.

We need to manufacture cars, with either petrol or diesel related parts (the parts can be the engine and the exhaust – e.g. different requirements for exhaust for diesel engines).

Engine
+ engineCapacity: Integer

DieselEngine: Engine

PetrolEngine: Engine
+ octanes (Integer)

We then have the car manufacturer interface:

public interface CarManufacturer {
    Engine createEngine(Integer engineCapacity);

    Exhaust createExhaust(Engine engine);
}

and its implementors

DieselCarManufacturer
PetrolCarManufacturer

There's the CarManufacturerGateway that orchestrates the car building process, selecting a CarManufacturer from a ManufacturersStore according to some criteria and then building a car via:

final var engine = carManufacturer.createEngine(engineCapacity);
final var exhaust = carManufacturer.createExhaust(engine);

Problem definition
To create the proper exhaust, in the PetrolCarManufacturer, we need to use the petrol engine's octanes number (probably a bad analogy) from within the PetrolCarManufacturer::createExhaust.

But, the CarManufacturer interface accepts an Engine, thus we cannot access the petrol engine octanes to decide upon the exhaust configuration to create.

Solutions

  1. instanceof + downcasting: I think it's considered a bad practice and can fail during runtime
  2. using double dispatch somehow?
  3. using parameterized types on the interface, e.g. public interface CarManufacturer<T extends Engine> and then each implementor will define the concrete type of engine it makes. This also requires the ManufacturersStore to be parameterized in the same way: ManufacturersStore<T extends Engine>. The latter makes me question this option, as it reads weirdly "a car manufacturer store parameterized on the type of engines it makes".

N.B.: If the analogy fails to demonstrate the issue at hand, I can try to rewrite it with our concrete example.

Best Answer

The problem here is that you've separated your concerns so much that you're unable to now implement a clear connection between the two. The engine and exhaust creation logic is so separated that you've forgotten what kind of engine you're working with when you get to the stage of creating the exhaust.

There are two possible scenarios here: you either need that separation, or you don't. I can't judge your scenario to that level of detail. This answer tries to answer both cases as best as it can.

instanceof + downcasting: I think it's considered a bad practice and can fail during runtime

Quick thought: your question implies that the right engine type must go with the right exhaust. So if you pass the wrong engine to the wrong exhaust factory, the logical consequence is that it should fail, specifically to honor your wishes that the right engine type must go with the right exhaust.

I agree that wanton downcasting is a sign of bad OOP, and leads to LSP violations. If you don't need the separation between the engine and exhaust creation logic, then it's best to couple them somewhat tighter so you don't lose track of the concrete engine type until you've also created your exhaust.

However, there are reasonable exceptions to this. For example, try catch uses downcasting in order to figure out if a thrown exception is of a given type or not. And this was necessary because the exception throwing logic needed to be as generic as it possibly could be.
To avoid upcasting in a try catch, you would need to strongly type your throw logic, which means having to custom implement it for your custom exceptions, which is not what you'd want. The alternative is pre-emptive upcasting to the base Exception type and then figuring out the concrete type at a later stage.

If you truly need the separation between the engine and exhaust, then it's acceptable to use downcasting here, and accept that when the wrong kind of engine is passed to the wrong kind of exhaust creator, that you run into an intentional exception there.
By having implemented that separation, you've signed yourself up to handle the concrete types yourself (since the typing system can no longer help you), and if you fail to handle it correctly and have no way to elegantly back out of the process, then an exception is the only option left on the table.

In this scenario, something did go awry in your application, since it's clearly trying to match an engine to the wrong exhaust creator. So if you take off the safeties (strong typing) and then don't handle things yourself, then a crash is inevitable and the right thing to have happen.

So, to summarize:

  • If you need (or cannot avoid) the loss of strong typing inbetween engine and exhaust creation, then downcasting is a valid solution to the problem.
  • Without that specific need, it is better to rewrite your code so that you don't forget the concrete type of the engine between its creation and the exhaust's creation.
  • Exceptions are not the devil. When you reach a bad situation, they are the correct response, to very clearly report to the developer that something major has gone wrong (in this case, an engine/exhaust creator mismatch)
Related Topic