open closed principle - refactoring to create base class based on new features

Question

So when original code was written there was only a need for say LabTest class. But now say we have new requirements to add say RadiologyTest, EKGTest etc.

These classes have a lot in common hence it makes sense to have a base class.

But that will mean LabTest class will have to be modified, lets say its interface will remain same as before, in other words consumers of LabTest class will not need change.

Is this violation of open closed principle principle? (LabTest is being modified).

Answer 1

I think the Open-Closed Principle, (as outlined by Uncle Bob anyway, vs Bertrand Meyers) isn't about never modifying classes (if software was never going to change it might as well be hardware).

& in your own case, I don't think you're violating OCP as you've mentioned all of your uses of your class are depending on the abstraction of LabTest rather than the implementation of RadiologyTest.

From Uncle Bob's introductory paper, he has an example of a DrawAllShapes class that if designed to OCP, shouldn't need to change each time a new subclass of Shape is added to the system. Regarding to what level you apply it, Uncle Bob says that —

It should be clear that no significant program can be 100% closed. For example, consider what would happen to the DrawAllShapes function from Listing 2 if we decided that all Circles should be drawn before any Squares. The DrawAllShapes function is not closed against a change like this. In general, no matter how “closed” a module is, there will always be some kind of change against which it is not closed.

Since closure cannot be complete, it must be strategic. That is, the designer must choose the kinds of changes against which to close his design.

I wouldn't read "closed for modification" as "don't refactor", more that you should design you classes in such a way that other classes can't make modifications which will affect you — e.g. applying the basic OO stuff — encapsulation via getters/setters & private member variables.

Answer 2

I may get burnt for this answer, but going on a limb anyways. In my opinion(IMO), OCP cannot be followed in the purist sense like other principles such as SRP, DIP or ISP.

If requirements change in such a way that you have to change the responsibility of a class to be true to their representation of the domain model, then we have to change that class.

IMO, OCP stops us from re-factoring code to follow the evolution of the system.

Please correct me if I am wrong.

Update: After further research, this is what I am thinking: Lets say, I have automated test both on unit level and integration level, then IMO we should redesign the complete system to fit the new model, OCP is out the door here. IMO, the goal of a system evolution is always to avoid hacks(not changing LabTest class and the corresponding DB table so as to not break old code[not violate OCP], and using LabTest to store EKGTest's common data and using LabTest inside of EKGTest or EKGTest inheriting from LabTest will be a hack, IMO) will be and to make the system represent its model as accurate as possible.

Answer 3

These classes have a lot in common hence it makes sense to have a base class.

I think you may be violating SRP. After all, if each class does one task, how can two or more be so similar? If there's a task they both do identically, then that is a separate task and should be done by another class.

So I would say, first refactor LabTest into it's constituent parts (hope you've got unit tests!). Then when you come to write RadiologyTest, EKGTest they can reuse the parts that make sense for them. This is also known as composition over inheritance.

But whatever you do, do use interfaces to these classes in the client. Don't force those who follow to use your base classes to add extensions.

Answer 4

I think you can look at it from two perspectives: existing requirements and new requirements.

If the existing requirements didn't cover the need for these kinds of changes then I'd say, based on those requirements, LabTest did not violate OCP.

With the new requirements, you need to add functionality that does not fit with the LabTest implementation. Adding it to OCP would violate SRP. The requirements now create a new change vector that will force you to refactor LabTest to keep it OCP. If you fail to refactor LabTest it will violate SRP and OCP. When you refactor, keep in mind the new change vector in any classes you create or modify.