How to properly apply open/closed principle on large code base for apparently unrelated features

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/398872

  •  02-03-2021
  •  | 
  •  

Question

Let's say I have a class hierarchy like this:

enter image description here

Now let's suppose I add later a new interface Mechanic, responsible for repairing a vehicle. Of course someone capable of repairing a car won't fix your broken bus. So you would have the BusMechanic and CarMechanic as implementations. My question would be, how do you create the right kind of mech ?

I see either :

The Vehicle interface could instantiate the right mech

I could add a method to the vehicle interface Mechanic createMechanic() which would create a new Mechanic instance with the right class and parameters

I find that it breaks SRP since that means that a vh has to handle its mech which is kinda nonsense.

Also it clutters the interface.

The Vehicle interface could provide the factory

Another possibility would be to add a method MechanicFactory getMechFactory(). This would fix the SRP issue, but I still don't really like it.

The Vehicle interface would still get cluttered

The chain of calls could get really long and not really readable, like customer.getVehicleFleet().getVehicleRegistry().getVehiculeForPlate(xxx).getMechanicFactory().createMechanic().getToolManager().getToolFactory().getRequiredTools()

The vehicle providing the factory for the mechs is still a non-sense

Using a map/enum

I could also map the classes implementing Vehicle with the right mechanic implementation like :

Map.of(Car.class, CarMechanic.class, 
       Bus.class, BusMechanic.class)

That would mean that someone subclassing a vehicle would also have to find all the relevant locations to map its class to the correct implementations

So far, the 2nd approach seems the better one, albeit far from ideal

Was it helpful?

Solution

There are two problems to solve:

  1. At design time, enforce that for every new vehicle type which is implemented, there must be also some code implemented which is able to create a corresponding mechanic.

  2. At run time, enforce, if a mechanic object shall actually be created, it is the correct mechanic type which corresponds to the type of a given vehicle object.

The first problem is solved by the classic Abstract Factory pattern, using an interface like

 interface VehicleFactory
 {
      Vehicle CreateVehicle();
      Mechanic CreateMechanic();
 }

It avoids any necessity to clutter the vehicle interface with methods which don't belong there. It still forces implementers to create factories which can always produce both types of objects, vehicles and corresponding "mechanic" objects. It is not easily possible to forget adding new mechanic derivation accidentally when a new vehicle type is added.

For solving the second problem, one has to know precisely how and where in the whole code base "mechanic objects" are created. Since you don't want to clutter your Vehicle interface with this, you need somewhere in your system a centralized factory method of the form

   mechanic = createNewMechanic(vehicle)   // returns  a corresponding mechanic

This centralized function then can internally use a map to find and call the correct factory object:

 Map.of(Car.class, carFactory, 
        Bus.class, busFactory)

The factory objects (of which there will be exactly one per type in your program) can register themselves in this map when they are created by a generic implementation in their base class constructor. This will make sure the map is dynamically kept up-to-date when new types of vehicles are added, without having to change the factory method for each new triple Vehicle/Mechanic/VehicleFactory. I guess this is probably the kind of "OCP design" you want to achieve: the source code of the factory method can be inside some "freezed" library, and a user of that library can still extend the "features" without changing the lib.

I am sure instead of implementing this kind of map and factory method by yourself, you could also utilize some DI container for this. I never tried it this way before, but I would actually be astonished if that's not possible.

OTHER TIPS

I feel the first option is reasonably find. I don't see adding of a method that produces additional object as breaking SRP. It would be breaking SRP if you added Mechanic's responsibilities as part of Vehicle.

I also don't see how this related to OCP, as OCP is about designing code that can be extended by adding code, instead of changing it. In your question, you are asking primarily about how to make a change once you know you need to make that change. Not about designing the code so it can be changed.

I think you are trying to mold your inheritance tree after domain business rules.

That will fail.

Eric Lippert can explain it way better than me. If you want to go in medias res, this is part 5 of his articles on that topic, but I encourage you to start at part one because it's really worth it.

The bottom line is: inheritance is a technicality that is supposed to help programmers. It is not meant to recreate your business rules. At some places it might help, but in others it will just spectacularly fail at the easiest tasks.

As long as you use inheritance for technical stuff (a button is derived from control, a NumberFormatter implements IFormater etc) it's great. But business rules are modeled along the real world with all it's quirks and exceptions, where sometimes a person (base class) can become a civilian or a soldier (derived classes in every simple school exercise) not by virtue of inheritance, but just by picking up a gun and a uniform "at runtime". Dropping the uniform might even make the same object a terrorist (or freedom fighter, depending on viewpoint). You cannot model that with compile-time inheritance. Your best bet might be composition or something else entirely.

So for your problem... why not make IMechanic a generic interface. IMechanic<T>. Now the implementer can decide if they do buses only, or cars, or all vehicles. Or you could have a non-generic interface and the implementer has a CanRepair(Vehicle v) method... because apart from type, there are a million other business reasons why a mechanic cannot repair a car. But don't be afraid to not be OOP pure at some place in your business logic components. Business logic does not care for your compiler rules. Those are for you, not the business. Just make sure that those places are restricted and don't bleed over the whole program.

Dependency injection can help you connect the right components. A method that lets you create a mechanic from a bus, screams like using OOP as a crutch, OOP for OOPs sake. You get a mechanic from a garage (you can call it a mechanic factory if you want) or by calling one. I have never seen a bus that somehow has a hidden mechanic compartment that I can open when I need one.

If you have a class that needs to repair buses, that class needs to get a mechanic for buses injected. Or a whole garage if the need for mechanics if kind of "lazy". But how mechanics are created or whether they have any relationship to buses are mostly business rules. Don't shoot yourself in the foot by forcing a OO relationship into the mix beyond what is required by the program logic. By required I mean if you can delete the code and your program still works, it wasn't required, it was just entertaining your perceived need for OOP.

Licensed under: CC-BY-SA with attribution
scroll top