5

I find it more convenient to 'modify'existing classes rather than extending them. Thus, I violate the open closed principle of 'not modifying the compiled and tested source code, instead extending the functionality'. I feel comfortable in modifying the source code because of unit test cases. Whenever I am modifying the code, I am confident that unit tests will keep me on track.

My question is, is it appropriate to modify the source code quite often if you have well defined unit test case. Or is it better to extend the classes without disturbing the original code?

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
Anmol Gupta
  • 175
  • 3
  • 10
    I think you have misunderstood 'closed for modification' it doesnt mean you cant modify the code, it means you cant inherit the class and modify the behaviour in the inherited class – Ewan Feb 20 '16 at 09:45
  • this topic seems to be thoroughly covered already, see eg [How do you unit test private methods?](http://programmers.stackexchange.com/questions/100959/how-do-you-unit-test-private-methods) and multiple questions linked to it – gnat Feb 20 '16 at 09:50
  • The link is about unit testing private methods? How is it related to this question? This question is about open-closed design principle. – Anmol Gupta Feb 20 '16 at 09:52
  • 1
    This one seems to be bit more related: http://programmers.stackexchange.com/questions/76384/how-to-apply-one-of-the-oop-concepts-closed-for-modification-and-open-for-exten but my question is about the practice which is followed while professional development – Anmol Gupta Feb 20 '16 at 09:54
  • 3
    Reading up more it appears that I am wrong and modification does indeed mean modifying the source! I think this is a good question about whether this principle is applied in practice and not a duplicate if the other linked questions. You could make it briefer though – Ewan Feb 20 '16 at 09:57
  • 1
    "How normal" and "is it preferred" are things that are often rooted in opinion and local code standards. –  Feb 20 '16 at 13:53
  • It is called "software" for a reason -- it is not cast in concrete, and changing it is sometimes the most appropriate way to add functionality. Too many devs think they can't modify existing code, opting for adding new code instead. – Erik Eidt Feb 20 '16 at 16:55
  • @Anmol: the topmost answer to that other question is a perfect example to your question, it is in no way special to "agile" methodology, and it explains well how the design of a class should be influenced by OCP through iterations, independently from having unit tests at hand or not. It is the missing piece to my answer, which explains the "use case" of the OCP", but not how to apply it. Do not just read the headline. – Doc Brown Feb 21 '16 at 08:55

7 Answers7

12

There is no "law" telling you all your code must follow the OCP. It is not even considered to be "best practice" to apply this to all of your code. This is a principle which is only applicable if you are going to build black box reusable libraries or modules, or a plugin architecture (I heavily recommend this article of Bob Martin, he calls plugin systems "the apotheosis" of the OCP).

The less you have to change an existing library to reuse it in a new place, the smaller are the chances something will break in other code which depends on that library. If the lib does not have to be modified at all, you cannot introduce any new bugs - that is nothing a unit test can guarantee. Tests can only reduce the risk for this, but they can never proof the absence of bugs.

However, making library code confirm to the OCP needs quite some effort, so it is always trade-off. When you are trying to build such a reusable piece of software, you will typically have to evolve it in several iterations (supported by unit tests) until it reaches the state where you do not have to change the code any more and can add new requirements (of a certain scope) just "by extension". To my experience, one needs at least 3 to 4 real reusage scenarios, compare them and think about the correct extension points for the lib to make it a "black box". Otherwise the library will never reach that state (YMMV). See this former question & answer for a detailed example how these iterations might look like.

So in short, the question should not be "OCP vs. unit tests". Use unit tests to keep your code evolvable, and modify it for new requirements until it reaches a quality where you do not need to modify it any more to reuse and extend it.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
3

There are two question there :

  • How often OCP should be followed.
  • How often OCP is being followed.

The first can be answered objectively:

Change is a risk in software development. If you know anything about risk management, you know that each risk has two components : a probability and a cost. As a software developer you should try hard to identify possible risks, their probabilities and costs. Now, changing existing code is expensive. That is because if code changes, then all other code that depends on the changed code needs to be recompiled, retested and revalidated. To reduce that cost, you can make the system or module open towards that change. That way, you can just add new code and old code remains unchanged, thus costing only testing and validating of new code. But there is one more cost : that is that making code open towards a change necessarily increases it's complexity and making code open towards one change might make it harder to make it open towards different change.

As such, it is on the developer to decide what changes have high risk of occurring and strategically make the code open to those changes. In your case, you can say that you are fine paying the cost of changing the existing code when specific change happens. And that is not a bad thing. But if you see yourself doing one kind of change over and over again, it might be good idea to make the code open towards that change.

Now, for the second question, this will get opinionated :

From all the code I saw, I would say developers don't make code open enough. There are many changes that are quite common in many projects and domains. Identifying those types of changes and adapting the design to them is crucial to keep the code maintainable. Yet, too often I see zero OCP even in cases where common changes are plain in sight.

Euphoric
  • 36,735
  • 6
  • 78
  • 110
2

"Code to interfaces" does not really mean you must create an interface with the interface keyword for every classes. What it means is that you should not depend on the implementation details of the class. Instead define a minimal set of characteristics/constraints that your client can expect will always be held by the class and any future versions of the class, this may be defined through the interface keyword but may also be purely in the documentation or a mixture of both. The users of the class should only depend on that public "interface", rather than the implementation detail.

Additionally, small (and sometimes not so small) classes whose usage is completely enclosed by another bigger class, may not need a documented interface at all. This internal class can be considered helper class of the main class, and is an implementation detail of the main class. If any consumer of the main class should not need to know what the internal class do or even if it exists at all, then it wouldn't really matter much to them if you do major changes to the internal class.

What "Open for extension but closed for modification" means is that a concrete class and future version of the interface may define additional promises beyond those defined by its documented interface; but they should not remove any of the constraints originally defined by the interface. This is so that a code that only knows about the interface would be able to use any concrete classes or future changes to the interface interchangeably and remain compatible as a class evolves and bug fixes or optimizations are added to a class and as the interface itself changes.

This usually means that you can add constraint and methods to an interface, but you should never remove them (or be really careful when you do).

Also, "closed to modifications" does not refer to modifications of the source code (implementation detail of a class), but rather to modification of the promises made by the class and/or interface.

how normal it is to modify the source code quite often if you have well defined unit test case

Very normal. As long as you don't break the constraints/promises of the interface, there is nothing bad about modifying the source code. A well-defined interface will allow you to modify the implementation freely, because your class and the consumer of your class have a well-defined agreement of what can and cannot be relied upon.

Changing the promises of the interface though, should be much rarer, and should require a bit more careful considerations. If it's an internal interface for an application code only you use, then you may be able to change the interface. But if you are writing a library or a framework, changing the interface may break other people's code.

When such breakage happens, there are two possibilities. If the breakage is because the other people is writing their code against your implementation rather than the interface, then that's their bug to fix; but if it's because your code doesn't fulfill its documented interface, then it's your bug to fix.

Lie Ryan
  • 12,291
  • 1
  • 30
  • 41
  • 3
    This is wrong. "Closed for modification" DOES mean not changing the source code. Because if you change the code, even if all the constraints are same, it still requires retesting and dependents to be updated. – Euphoric Feb 20 '16 at 10:59
  • 1
    @Euphoric: if what you said is true, then that means you should never fix bugs in any code, because any changes in a code can potentially break any dependants. – Lie Ryan Feb 20 '16 at 11:23
  • Like I said in my post. It is always about cost. If effort of fixing the bug is lower than what is gained by having the bug fixed, then it is worth it. And it is generally more expensive to change code with lots of dependents, than code that is independent. – Euphoric Feb 20 '16 at 11:29
  • 1
    The Linux kernel is a code in which pretty much every program in your system depends on it. Yet, the kernel managed to totally rewrite how core OS interfaces like forking or memory allocation was implemented, with relatively little hitch to libraries and userspace programs. How did it manage to do so if changing the text of source code is as expensive as you made it out to be? – Lie Ryan Feb 20 '16 at 11:51
  • Programs (irrespective of OS) depend on the APIs provided. Only small amount of applications actually depend on the kernel itself. In that sense, the applications are open towards changes in the kernel. This design is a deliberate decision by Linux developers, stemming from experience. I would say that applications for older OSes (MS-DOS for example) wouldn't be so forgiving to changes in OS kernel. – Euphoric Feb 20 '16 at 13:48
  • @Euphoric, the answer is a little wordy, maybe even misleading at times, but it's not outright wrong. You can definitely fix, optimise, refactor, extract code under the hood; otherwise, you'd end up with a new implementation for every bug fix and all the common code duplicated. The interface is by far the most important thing. But, interface is a misleading term in this context, because of the implementation of some languages. Interface includes things like "I return an empty string if not found" which should never change to "I return a null if not found", even if it makes more sense. – pdr Feb 20 '16 at 14:33
  • 1
    -1, Euphoric is right, you mixed up the OCP with "design by contract". The OCP *is* about not modifying the source. However, there is no need to have each and every component in a system to follow the OCP to create a high quality system like an OS. – Doc Brown Feb 20 '16 at 15:54
2

I guess this is a pretty complicated question, where we might do different things in different scenarios.

1: 'Business As Usual' new requirements for specific use case: You would expect a new child class, or interface implementation with no modification to the base class, and in my experience this is generally what is actually done.

2: BAU requirements change over time for original use case: so here you could argue that the correct approach is to leave the original class and have V2 class with the changes in. However mostly I see the original class (and unit tests) being modified and versioning achieved through source control.

3: new project with evolving requirements. In this case I think everyone modifies the original class, it's still in development after all. There is a bit a grey area between a new systems release and it being seen as a 'bedded down' working system.

4: A third party library needs customisation. This is less common but I've seen it a few times where open source libraries which are not extendable enough get their source downloaded, modified and included in the solution. This achieves the goal but obviously creates a maintenance nightmare going forward.

I think you are right to call out unit tests and testing in general as a major factor when considering what code to change. Systems with a strong set of unit tests give you much more confidence when changing code and make it easier to modify existing classes rather than create a new implementation (with associated tests)

In established systems that are still in-use, there can be a great deal of risk adverseness in deploying changes. There will often be questions asked about 'what needs retesting.' If your change only requires parts of the system to be redeployed, then this is much favoured. If only due to the time and expense of retesting the 'whole system!'

Lie Ryan
  • 12,291
  • 1
  • 30
  • 41
Ewan
  • 70,664
  • 5
  • 76
  • 161
2

This is an dddition to Lie Ryan's answer.

As with pretty much anything in programming, even here the approach will not be similar for different cases.

You are justifying the modification procedure by having well written unit tests, but even then, directly modifying a class or making it open for extension usually depends on what exactly you are currently programming.

Case I.

When it makes sense to modify an existing class

Imagine you have your domain layer containing the business logic of your application. In an ideal world, the business rules are well defined and are set. There is only one set of the business rules.

In a situation such as this, you will rarely have different implementations of a similar thing, you are limited by the business constraints and these have to be well defined.

You will probably have unit tests for this layer and when you modify this layer, you want the tests to fail, inidicating something (usually a business rule) really has changed.

Case 2.

When it makes sense to extending a class

Then there's the other situation, which can be applied to pretty much anything aside your domain. Services, modules, persistence layer, caching layer, repositories,...

Many of these are classes, which, even though may only exist once in your code, maybe replaced one day with a better alternative. This is when it is good to code to an abstraction, be it an interface or an abstract class acting as a supertype for concrete implementations.

By not depending on a concrete implementation but an abstraction, when a new alternative for said service (be it caching, persisting or processing data) comes, you can simply provide a new implementation, change your configuration to use the new implementation and you are pretty much set, without toouching the rest of your (usually pretty large) codebase.


OCP definitely does not make sense for everything. As with every pattern, even this one is meant to ease the development but is not a rule to be sctirctly obliged by.

Andy
  • 10,238
  • 4
  • 25
  • 50
1

The open close principle is important when creating public APIs or libraries other people will use. Situations where you have no or less control over the calling code make interfaces changes a pain, therefor you should try design them such that you can avoid most of them.

What potential 'modifications' are though can become too speculative, risking troubles with the YAGNI principle (which I find far more important).

When you also control the calling code and you have covering tests I would not give this principle too much consideration.

Joppe
  • 4,548
  • 17
  • 25
0

The OCP principle primarily applies when there is code and applications which you don't control which uses your library in production. Say you develop a library or framework or service which is used by many different clients. You don't know all the different ways your code is used by these clients. How will you know if your change breaks something?

You can unittest you own code thoroughly to be confident a change will not break anything in your own code, but you can never be sure a change don't break some other peoples code in another system. They may rely on some obscure edge-case behavior you never even thought about. This is a very real concern when developing a widely used library or framework.

If your library is only used by code you know and have access to, then changing rather than extending is usually better, since it keeps the code smaller and simpler.

JacquesB
  • 57,310
  • 21
  • 127
  • 176