3

I try to write "clean code" for most of the time. But practically find it very hard, meaning - gradually business requirements changes dramatically or the business requirement which seems like just a condition force to modify existing code.

If we go for "extend" instead of "modify" approach, implementation seems clean. But feels like to much class hierarchy for use cases which seems simple. Moreover, sometime just writing few lines of code with "modify" fix the problem in given "Time" instead of writing few set of classes with "extend" approach.

How to ensure to keep code clean not when it is implanted for first time, but few years down the line. Specifically when time is critical and for business new requirements are "JUST a small change".

Bhalchandra K
  • 235
  • 1
  • 7
  • 5
    The best answer I could give you is 100% identical with [this former answer of mine](https://softwareengineering.stackexchange.com/a/310618/9113), though the question was literally different. – Doc Brown Dec 21 '17 at 06:40
  • Possible duplicate of [How can I convince management to deal with technical debt?](https://softwareengineering.stackexchange.com/questions/43948/how-can-i-convince-management-to-deal-with-technical-debt) – gnat Dec 21 '17 at 07:42
  • 1
    "when time is critical" just because business says it is critical doesn't make it critical. Way too often, business people over-exaggerate impact of possible change. If you are in organization, where IT is treated withou respect to their work, then you have much worse problem than following OCP. And no technical solution can help you. – Euphoric Dec 21 '17 at 07:46
  • 1
    @Euphoric I agree with you there. Everyone *except* the actual programmers writing the code think that the time it takes to fix a boat leak is the time it takes for you to plug up the hole. That neglects the fact that there are "throw tar on the hole" fixes and then there are "replace the planks and fix it better than before" fixes. To achieve the second, you really do need time and attention, that you're sometimes not given. Only you and your team can explain this idea to your boss. – Neil Dec 21 '17 at 08:13
  • 7
    @gnat: not even close. This question has **nothing** to do with convincing management about anything. – Doc Brown Dec 21 '17 at 09:56
  • 1
    Regarding changes and evolutions, you should not consider OCP in isolation, but together with SRP, the Responsibility being about the reasons to change. – Christophe Dec 21 '17 at 13:54
  • @gnat FWIW, I think a lot of these "Do I really have to do , it doesn't seem to help!" questions may be from a certain set of (low quality, IMO) published media that rather aggressively implied that code written in certain languages that was built on anything other than OCP could only be a mess and/or poorly designed; I can't remember exactly where this came from or find them now, but I remember this coming up very often from Google searches a few years back, so either the pages have fallen off of Google's ranking algorithm, the blogs have been taken down, or I'm not using the same queries. – jrh Dec 21 '17 at 15:10
  • 1
    People still talking about the SOLID principles, are people still stuck in the 90's. I mean, are we still talking about Open/Closed, still? Let's all form a posse and run screaming out of this inheritance-over-composition nightmare together. – code4life Dec 22 '17 at 13:51

3 Answers3

5

Don't follow the Open/Closed principle (OCP) unless you absolutely have to. In most application development OCP is not appropriate and only leads to overly complex code, as you have already noticed.

OCP is appropriate in scenarios where you have independent, external clients using your code, and you need to evolve the code while preserving backwards compatibility. Take the .net framework: MS cannot just modify the behavior of an existing API in the framework, because that would break all the client depending on the current behavior. They can only extend the API. OCP does not actually lead to clean code - for example the frameworks have multiple deprecated API which can never be removed. But it preserves backwards compatibility.

Another scenario is if you have a "big-ball-of-mud" architecture where each change may have unexpected side effects across the system, and no automated testing to protect against regressions. In that case you may adopt a paranoid mindset, and keep the code "closed" to avoid breaking existing code. But here not to modify the code is only treating the symptoms, and will not in itself improve the quality of the code. If possible, you should prefer to treat the root cause of the problems.

If you are not forced to follow the OCP principle then it is often cleaner and simpler to modify behavior rather than extend it.

Like all programming guidelines, OC is not an universal rule but is only appropriate in specific contexts.

JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 2
    I agree with this answer. And it makes me wonder why SOLID is so highly rated, when it's #2 principal, OCP, is generally meaningless, it's #3 principal, LSP, seems to mainly affect mutable rectangles and squares, and it's #1 principal, SRP, is frequently misunderstood and misapplied. – user949300 Dec 21 '17 at 08:37
  • 2
    "It is often cleaner and simpler to modify behavior rather than extend it.": I disagree with this. If you modify the behaviour of a module that is used internally by many other modules, you can have unexpected side effect and changes of behaviour everywhere in your software. I have seen so many bugs appear because of this. If possible, you should try to stabilize your code over time, especially those modules on which many other modules depend. – Giorgio Dec 21 '17 at 08:46
  • 4
    @user949300: I don't think OCP is generally meaningless, it is just that you need to understand the contexts where it is useful. The problem is not the principles themselves, the problem is people applying them uncritically. – JacquesB Dec 21 '17 at 09:28
  • @Georgio: Good point. I have added a paragraph than OCP might also be appropriate if you have a legacy "big-ball-of-mud"-architecture. – JacquesB Dec 21 '17 at 11:31
  • 3
    @JacquesB `the problem is people applying [principles] uncritically`: when enough people misapply a principle, the fault lies with the principle (or at least, with how it's explained). The same happens with statements such as "methodology X would work, if only": when the "if onlys" become boundless, it's a sign the methodology doesn't work outside laboratory conditions ;) – Andres F. Dec 21 '17 at 15:45
  • @AndresF.: Good point. – Giorgio Dec 21 '17 at 18:30
  • 2
    @JacquesB: your answer has some good points in it, but when you wrote the part about "big-ball-of-mud" systems, you were intermixing "following the OCP" (which is something the author of a component does to prevent the necessity for code change)s with "not changing existing code" (which is something a **user** of a component decides about, but has IMHO *nothing* to do with the OCP). The OCP is a design principles for designing reusable components, not a principle for reusing code in a "don't change" manner (and so producing technical debt). – Doc Brown Dec 21 '17 at 20:37
  • ... so maybe your initial recommendation for not following the OCP is caused by the the same misunderstanding? Maybe you wanted to recommend to apply changes to existing code, and not to avoid following the OCP? – Doc Brown Dec 21 '17 at 20:39
  • 1
    @DocBrown: Good point, I could have been more clear. My point is that the OCP make the most sense when the designer of the component and the client are independent. This is the case with a 3rd party library or service API, but not when the component and client code is in the same project, in which case OCP is not necessary. But if you have a big-ball-of-mud architecture, it might still make sense to treat components as closed, out of fear of breaking anything if the component is changed. – JacquesB Dec 22 '17 at 07:35
  • @JacquesB: as I wrote in my answer I linked to, I agree mostly what you wrote about OCP for 3rd party libs. However, I think your usage of the term "OCP" as a synonym for **treating** components as unmodifiable (as opposed to **designing them** ) is 100% wrong. So what are you recommending here in your first sentence? Not trying to design components in that way, or not trying to reuse components without modifying them? Your partly correct, partly wrong usage of the term OCP makes your answer look very strange to me. – Doc Brown Dec 22 '17 at 07:47
  • @DocBrown: I recommend designing components as "open for extension" if you are writing a component/library for use by third parties. Otherwise follow the YAGNI principle. – JacquesB Dec 22 '17 at 08:28
  • @JacquesB: that is fine, but still I recommend to rewrite (or delete) your paragraph about the "big-ball-of-mud" architecture from your answer. You are pretending there that keeping legacy code unchanged is a form of applying the OCP, that is IMHO wrong. If you would write instead that some people mistakenly think that has this something to do with the OCP, then your answer would IMHO become better. – Doc Brown Dec 22 '17 at 09:14
  • @DocBrown: I don't know if there is an "official" definition of OCP, but Robert Martin states "*The openclosed principle (..) says that you should design modules that never change. When requirements change, you extend the behavior of such modules by adding new code, not by changing old code that already works.*" (https://www2.cs.duke.edu/courses/fall07/cps108/papers/ocp.pdf) Of course you may disagree with this interpretation of the OCP, but it is not just me that has misunderstood the principle then. – JacquesB Dec 23 '17 at 23:20
  • @JacquesB: well, may understanding of this cite is *you should design modules [..], so - you don't need to - extend the behavior of such modules [...]*. So Uncle Bob is perfectly in sync with what I wrote above - the OCP is a design principle, not a reuse principle for components which are misdesigned. – Doc Brown Dec 24 '17 at 08:20
  • ,,, and if you look at the whole paper, without taking single sentences out of their context, this is all about how to design components, not more. Or if you look at the other SOLID principles - they are all **design** principles. Those sources don't tell you how to use, reuse or abuse components, they only tell you how to design components. But to be fair, the words Uncle Bob used there in his *"you are not allowed"* style are open for misinterpretation (but not closed for thinking twice about their meaning) ;-) – Doc Brown Dec 24 '17 at 08:27
  • @JacquesB: but you are not alone, that fundamental misunderstanding was already discussed in this former [SE.SE question](https://softwareengineering.stackexchange.com/questions/348102/why-do-many-software-developers-violate-the-open-closed-principle/348112#348112), for example. – Doc Brown Dec 24 '17 at 09:48
  • @DocBrown: I don't see how discussion about designing components according to OCP can be separated from discussion how to use such components (extending vs modifying). I certainly don't see the value of deliberately ignoring one side of the coin. Understanding the *purpose* of the OCP helps one understand when it is appropriate to apply. The Martin quote is about the *purpose* of the OCP, not a *definition* of the OCP. – JacquesB Dec 24 '17 at 10:35
  • @JacquesB: yes, it is about the purpose, and not about the defintion. That is exactly my point. The purpose of OCP is that code changes become unnecessary, but the OCP itself, its defintion, refers to the design process . In your answer, however, you still write "applying the OCP" synomously to "not changing code of a big-ball-of-mud system". And that is IMHO wrong. – Doc Brown Dec 24 '17 at 11:15
  • @JacquesB: suggestion, I will edit your answer slightly to correct this error, and if you don't like the edit, you can revert it. – Doc Brown Dec 25 '17 at 07:35
5

Do you expect OCP to prevent you from changing the code on all business changes? When requirements change from "give me an application that will lend bikes" to "give me an application that will steal cars" no rule will save you.

So why should we follow OCP? Why is it useful? I think Strategy Pattern is the simplest way to explain the power of OCP. Let's consider part of the system that calculates discount for some selling process. Let's assume that discount depends on order value as well as on the user that buys (VIP, regular etc.). One approach is:

public calculateDiscount(Customer customer, Order order) {
    // some common part

    Discount d;

    if(customer.isVip(){
        d = // some code
    } else if(customer.isRegular()){
        d = // some code
    }
    // etc.

    // some common code
}

another one:

public calculateDiscount(Customer customer, Order order) {
    // some common part

    DiscountPolicy dp = customer.getDiscountPolicy();
    Discount d = dp.calculateOn(order);

    // some common code

}

What if there will be another type of Customer? First approach forces you to modify code. Will this change break behaviour of calculateDiscount? Will you be able to make modification without introducing errors? I know, in this simple example probably yes ;)

But there are many parts of your system that you already know or strongly feel that will be modified or extended (not in meaning of inheritance, but in meaning of covering another nuances of use cases) in the future. You have to prevent your code from errors as much as you can. One of the method of such prevention is not changing the code that works. OCP is all about not changing code that works. Strategy, Decorator, Abstract Factory, Command etc. patterns are all about keeping some part of code in compliance with OCP.

Obviously, there are changes that you are not prevented from. Adding requirement to calculate discount depending on market type also will end up in changing contract and code inside calculateDiscount method as well as unit tests code and other tests that you have. It is inevitable. But OCP gives you the "crumple zone". Like in real world - given crumple zone is not always sufficient for crashes we come across.

krzys
  • 151
  • 2
4

Following the OCP should always be one of the goals of an OOP programmer. Ignoring this principle erodes how useful it is to decompose into objects. It is exceptionally lazy to think, "Well if they want to react to change they can just rewrite it".

OCP asks you to favor a design that permits change by adding new code rather than changing old code.

The structural mechanisms for this can as complex as a design pattern or as simple as introducing a variable. Abstraction works best when you can't see the details you're using.

The most telling problem is when you see fit to separate one idea from another by putting them in different objects yet one KNOWS exactly which implementation of the other it's talking to. Doing this not only violates OCP but it means you typed up an extra class for no good reason. If you separate ideas into different objects they should not hold each other in a death grip.

However, change is difficult to predict. Yagni teaches us not to create things that might be useful, only things that are useful today.

Rather then predict the future, be conservative when you assume something will be stable. Our high level abstractions, our interfaces, really hurt us when they change. Keep what they assume to be stable to a minimum. Push what you're not sure of down to lower levels. Break them up to keep their vulnerability to change footprint small. Let them serve only one master.

Do this, and following OCP shouldn't be too dramatically different. If, however, you go nuts slapping interfaces on every object and refuse to call anything that isn't polymorphic, well you're giving us OOP coders a bad name.

The best advice you could apply here was actually about when to react to a DRY violation. You are far more likely to decompose correctly the second time you repeat yourself then you are the first time. So maybe don't be so quick to react.

This wisdom should temper how quick you are to predict change. Unit tests help us see how we could change implementation details. While this is a good structural exercise unit tests aren't production code. Be careful thinking they show you how things will change.

But you should feel bad every time you add a feature or fix a bug by changing existing code. Even if you don't actually have independent external clients it is really nice when you can support large swaths of your code as if it were.

Remember, these principles are not going to help you get your code to work any faster. They help you keep your code working once it does. Therefor you wont find out if you're wasting your time putting effort into following them until changes start coming in. If you want to practice applying them then you can't just code to a fixed specification. You have to code to a changing specification that surprises you.

A conservative way to apply these principles is to add complexity in reaction to change rather than in anticipation. When I take this tract I'll forgive myself for having to change existing code in a module once, maybe twice. After that it's time to see about stopping that from happening again.

Otherwise we might as well go back to procedural programming because even it can deal with change if you're willing to rewrite every time.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • *"A conservative way to apply these principles is to add complexity in reaction to change rather than in anticipation."*- to my experience, that is the only way which works well, and I don't think it is conservative, that is actually what YAGNI is about. – Doc Brown Dec 21 '17 at 17:15
  • @DocBrown better? – candied_orange Dec 21 '17 at 20:02
  • 2
    Well, I think your answer is still neither really bad, nor really good. First, I am convinced the OCP is only a good fit for reusable, generic parts of a program. Trying to apply it to every part instead of generic parts leads easily to overengineering. To be fair, this kind of advice is IMHO equally bad as the other extreme in the currently accepted answer, to avoid applying the OCP. ... – Doc Brown Dec 21 '17 at 20:12
  • ... second, your answer does not give the OP a clue that inheritance and polymorphism is not the only way to make a class or component more "open for extension". Sometimes a simple parameter or configuration option is all one needs. More important, often decomposing a component into smaller pieces with restricted responsibilities is the best way to implement the OCP. I am sure the OP has not understood this, that is part of his objections against it, and I am under the impression your answer leaves him in this misbelief. – Doc Brown Dec 21 '17 at 20:21
  • @DocBrown how about now? – candied_orange Dec 21 '17 at 20:33
  • Not really, only a little bit, I don't see my points adressed. I gave you already an upvote, I think your answer is somewhat better than the accepted one. I still think if I would write an answer here, I could only repeat what I wrote to that other question I linked to in my initial comment, and that is something I won't do. – Doc Brown Dec 21 '17 at 20:45
  • @DocBrown Well thanks for the upvote but I'm still curious about how I could improve. I thought I hit on there being many ways to extend and decomposing. – candied_orange Dec 21 '17 at 20:48
  • If I would go even more into details, I would probably recommend to rewrite large parts of your answer, but that is surely not worth the hassle. And I think we should stop this discussion here, took me already more time than I expected. – Doc Brown Dec 21 '17 at 20:55
  • @DocBrown: disagree with your point about OCP being possible through a simple parameter or configuration. That's not what OCP is talking about at all. Bertrand Meyers (yes, the man who invented the term) defines OCP as the base rationale for object inheritance. Another words, he explains that object inheritance was invented to solve this dilemma. So no, a simple parameter or configuration is not what OCP is talking about. OCP is dealing with the question of, how do we extend the capabilities of a closed (aka compiled) module (e.g. DLL) in the most efficacious manner. – code4life Dec 22 '17 at 19:01
  • @code4life: the problem with the OCP is that it is badly explained in literature, even by people like Betrand Meyer. See my former answer here: https://softwareengineering.stackexchange.com/questions/361977/is-it-a-violation-of-the-open-closed-principle-to-update-a-constant-representing/362033#362033 – Doc Brown Dec 22 '17 at 23:04