1

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).

krprasad
  • 359
  • 1
  • 4
  • 17

4 Answers4

2

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.

brader24
  • 485
  • 2
  • 10
1

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.

weston
  • 54,145
  • 21
  • 145
  • 203
  • In a way we are saying, that OCP does not play well when expectations(requirements) from the system change. If I was to purely follow OCP, I am supposed to leave LabTest alone and duplicate all the logic/code which will be used in RadiologyTest and EKGTest, which breaks DRY principle. – krprasad May 02 '14 at 13:54
  • 2
    Well I'm saying because you didn't use the other SOLID rules, you have a large class that feel bad about duplicating. But you don't have a crystal ball, so this will happen (i.e. often some part of the system needs to be extended that you didn't forsee). Best thing to do is refactor first (using your test suite) to make the system extendable in that regard, and then extend it. Then when it happens a second or third time, you're ready for it. – weston May 02 '14 at 14:10
  • 2
    @weston I think this gets at the heart of what I tried to say. You never have a crystal ball. OCP does not say that you should never change a class, especially when requirements change, but that when you realize that a class needs to change that you refactor it in a way that it **does meet** OCP for future changes of a similar type. And I agree, you should refactor first to make the code meet OCP for the given type of change and then extend. – brader24 May 02 '14 at 14:34
1

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.

krprasad
  • 359
  • 1
  • 4
  • 17
  • I will let the moderators decide the correct answer. I have no issues with any choice since the doubt in my mind did get cleared due to this discussion. – krprasad May 05 '14 at 13:46
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.

anotherdave
  • 6,656
  • 4
  • 34
  • 65
  • huh, a rule with undefined exception. So how do we decide when a OCP is violated? As I said, it is one of the rules to which I am going to pay least amount of attention. As long as we follow the other design principles and good amount of common sense we should be fine. – krprasad May 12 '14 at 19:39
  • I think that it may need more flexibility that trying to apply it in a black & white fashion (has it been violated - yes/no). As with all design principles, they're not about needing to follow them religiously, but more giving you a heuristic to go by. I'd tend to apply OCP to changes I think are likely in initial design & to similar changes, when refactoring for an unforeseen requirement. This blog post is also worth a look: http://blog.8thlight.com/uncle-bob/2013/03/08/AnOpenAndClosedCase.html – anotherdave May 12 '14 at 22:06