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

  • 5
    The best answer I could give you is 100% identical with this former answer of mine, though the question was literally different. – Doc Brown Dec 21 '17 at 6:40
  • 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 7: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 8:13
  • 7
    @gnat: not even close. This question has nothing to do with convincing management about anything. – Doc Brown Dec 21 '17 at 9: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
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.

  • 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 8: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 8: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 9:28
  • 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
  • 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
4

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.

  • Nice answer, pragmatic. Maybe a slight rewording would improve it. "OCP, applied pragmatically, not dogmatically is all about not changing code that works". Welcome! – user949300 Dec 22 '17 at 7:00
3

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.

  • "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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.