Extracting interface or use double dispatch to avoid downcasting?

Question

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.

Answer 1

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)

Answer 2

One option would be to introduce another layer of abstraction

public interface CarManufacturer {
    PartFactory createPartFactory(Integer engineCapacity);
}
public interface PartFactory{
    Engine getEngine();
    Exhaust getExhaust();
}

This should guarantee that the exhaust matches the engine. The downside is more interfaces and a bit more complexity.

I do not see how double dispatch would solve this problem, while you avoid the downcast, you still need to handle the case where a diesel engine is passed to a petrolManufacturer, and this would still be a runtime error.

While I agree that runtime errors should be generally avoided, the overall goal is to make it as easy to use as possible. In some cases that might very well be done by clear documentation and runtime error messages rather than by designing overly complicated interfaces.

I have used some APIs where just about every method started with something like if(obj is not SomeInternalType) throw "invalid object type, object has to be created by CreateObject method" without any real issue. After all, it is much easier to call the CreateObject method provided by the API than to create my own implementation of the interface.

Answer 3

How does the PetrolCarManufacturer know what octane number to give to the PetrolEngine it creates? The octane number is not part of the CarManufacturer interface, so the PetrolCarManufacturer class needs to have the knowledge in some other way (member variable, read from some configuration object, etc.).

As the PetrolCarManufacturer already needs to have knowledge of the octane number it uses to create PetrolEngine objects, it can use that exact same knowledge within the createExhaust method.

The assumption here is that a single PetrolCarManufacturer can only create Engines with all the same octane number and that all parts that need to go together are created by the same factory.