Is the Open/Closed Principle a good idea? [closed]
Asked Answered
S

6

18

This question is not about what OCP is. And I am not looking for simplistic answers, either.

So, here is why I ask this. OCP was first described in the late 80s. It reflects the thinking and context of that time. The concern was that changing source code to add or modify functionality, after the code had already been tested and put into production, would end up being too risky and costly. So the idea was to avoid changing existing source files as much as possible, and only add to the codebase in the form of subclasses (extensions).

I may be wrong, but my impression is that network-based version control systems (VCS) were not widely used back then. The point is that a VCS is essential to manage source code changes.

The idea of refactoring is much more recent. The sophisticated IDEs that enable automated refactoring operations were certainly inexistent back then. Even today, many developers don't use the best refactoring tools available. The point here is that such modern tools allow a developer to change literally thousands of lines of code, safely, in a few seconds.

Lastly, today the idea of automated developer testing (unit/integration tests) is widespread. There are many free and sophisticated tools that support it. But what good is creating and maintaining a large automated test suite if we never/rarely change existing code? New code, as the OCP requires, will only require new tests.

So, does the OCP really makes sense today? I don't think so. Instead, I would indeed prefer to change existing code when adding new functionality, if the new functionality does not require new classes. Doing so will keep the codebase simpler, smaller, and much easier to read and understand. The risk of breaking previous functionality will be managed through a VCS, refactoring tools, and automated test suites.

Selfgovernment answered 12/9, 2009 at 23:44 Comment(8)
Lengthy comment thread that lead to this question: #1379730Gregarine
It appears as though you're simply looking for an argument, and that you've already made up your mind.Shores
You formulate a question, but you answer it yourself in a way that doesn't really even encourage a debate. This should possibly be closed as "not a real question", and if not that then definitely made community wiki.Arethaarethusa
Version control systems were in use in the 70's. Refactoring to change 1000's of lines of code is still a challenge for source code management systems. Having a test does not any better justify changing code than having a seat belt justifies having a car accident.Unappealable
This is a good question. He is saying that he was under the impression ZOCP it is mostly in existence because it is dangerous to change production code - the question is whether that is true.Gregarine
September 16th 2009 The virtual alt.net group will be discussing this very principle. Just join in: virtualaltnet.com/Home/Calendar Recordings usually become available about a month after the meeting.Gregarine
It's unbelievably stupid that this question was closed. Another StackOverflow FAIL.Rain
OCP is about class invariants guarantees. There I said it all in 3 words.Murchison
R
12

OCP makes a lot of sense when you aren't the consumer of your code. If I'm writing a class, and I or my team am writing all of the classes which consume it, I agree. Refactoring as things change is no huge deal at all.

If, on the other hand, I am writing an API for my customers, or I have multiple consumers in a large organization with varying interests, the OCP is critical because I can't refactor as easily.

Also, if you just refactor your class to meet everyone's needs, you'll get a bloated class as a result. If you designed the class to allow consumers to extend your class rather than modify it, you wouldn't really have this problem.

Rain answered 12/9, 2009 at 23:59 Comment(3)
Yes, but you are talking about changing a published API. In this situation you obviously are much more limited in what changes can be made to the public interfaces, OCP or not. But notice that you could still safely change the implementation behind the published API. The problem is that the OCP does not make any distinctions, but simply prohibits modifications on the assumption that they would be unsafe.Glaikit
I really think applying OCP only to APIs is a bit of a misunderstanding. In an application where development has not yet stabilized (ie 95% of all features that ever will be done are done) essentially every part of it is an API - OCP applies to continuous development without having to change every component that uses you.Gregarine
The point is that an internal (non published) API can be modified almost just as easily as the encapsulated implementation details. BTW, "published API" is a term coined by Martin Fowler to describe public APIs that are published for use by unknown third-parties (just to make sure everyone understands what I am referring to).Glaikit
L
6

I never heard of OCP being that. Maybe you are refering to something else, but the OCP I know says "A module/class must be open for extension, but closed for modification, meaning that you shouldn't modify the source code of the module to enhance it, but the module or object should be easy to extend.

Think of eclipse (or any other plugin based software for that matter). You don't have the source code, but anyone can write a plugin to extend the behaviour or to add another feature. You didn't modify eclipse, but you extended it.

So, yes, the Open/Closed principle is indeed very valid and quite a good idea.

UPDATE:

I see that the main conflict here is between code that is still under development and code that is already shipped and used by someone. So I went and checked with Bertrand Meyer, the author of this principle. He says:

A module is said to be closed if it is available for use by other modules. This assumes that the module has been given a well-defined, stable description (its interface in the sense of information hiding). At the implementation level, closure for a module also implies that you may compile it, perhaps store it in a library, and make it available for others (its clients) to use.

So, indeed, the Open/Closed Principle refers only to stable, ready for compile and use entities.

Loram answered 13/9, 2009 at 1:38 Comment(13)
+1 This is the essence of what OCP is about.Practicable
I understand what you say. I also agree that modules that are published for external consumption should not have their interfaces changed. This is quite obvious, actually (although it sometimes happen in the real world, for example in the ASM project). But note that the definition you quoted does not say "published module". My problem with OCP is just that, it seems to forbid changes to internal modules. And most code I ever developed was internal only, not published.Glaikit
Put another way, if it is true that "the Open/Closed Principle refers only to stable, ready for compile and use entities", then it's not a very useful principle. Consider a typical custom-made business application, which is what most software development projects are about (for me and most developers I know, anyway). In such a codebase, nearly all (often all) modules are internal, non-published, and hardly "stable". Conclusion: in such projects the OCP does not apply.Glaikit
Well, business applications do have a stable part and that is how the business runs. Rules may change, but in any domain there are specific parts that remain unchanged, just because they are specific to that business part. You should find that commonality and have them stabilize into an interface. What changes, should only be extensions to that. There is another principle that says more or less "Localize what changes".Loram
Here's an example from document printing. Documents go through a lot of processes: creation, formating, proofing, printing, putting holes in the paper, stitching etc. Not all documents go through all of these. So you create a workflow infrastructure and you only change what processes go into that workflow and in what order. Voila! I've just applied the Open/Closed principle. Nothing published, no APIs, not even code. I've just talked about architecture in my business domain. I'm sure you can apply that in your domain, too.Loram
I think you are falling for the misconception that if something is (to you) obvious, it can't be what someone meant, so it must mean something else instead.Venn
@Andrei, the OCP clearly talks about working code, not about architecture or infrastrusture. The fact that a given class remains unchanged for years does not imply that it "has been given a well-defined, stable description". Quite the contrary: just as a business analyst can modify a use case specification at any time to reflect changed business rules or user-system interactions, the internal classes that realize the use case can and will be modified accordingly. How is OCP relevant when all classes are internal? And who needs a Principle to know that a published API should be stable? I don't.Glaikit
Well go and implement that architecture. Doh! The OCP can be applied at different levels. A class that remains unchanged for years is stable! You shouldn't worry now that it will change 10 versions in the future. Use cases shouldn't be implemented by distinct sets of classes. Like I said, you should build a common infrastracture that is relatively stable, and its extensions should be modified as a use case or the others change. And to give a definite answer to "How is OCP relevant when all classes are internal?" OCP is a way to organize those classes so that you cand deal with changes.Loram
The official OCP articles do not describe different levels of applicability, I believe. I don't think we should try to "reinterpret" published concepts to fit our personal needs. They are what they are, not what we now want them to be. You are correct that OCP is "a way to organize classes to deal with changes", of course. But it's a bad, over-engineered, ineffective way. That's what this question is about.Glaikit
But you are reinterpreting it by dismissing it. It wasn't what I wanted. It's a principle, and I applied it. I could possibly discover that some system found in nature would respect it if I knew where to look. You admited yourself different levels of applicability by (rightfully) moving the discussion from published APIs to internall classes. Anyway, you make it sound like a methodology, but it's not. It's a principle. And a principle exists because it gives you a hint that this piece of software has a bit of quality. And I've given you at least two examples where this happens.Loram
In software, as there are things that change a lot, there are also things that don't. Close the ones that don't. If at one point it crumbles down, reopen it. Also note that modifiying a module doesn't mean not changing the code. It means not changing the interface. You can change the internals all you want, bring optimizations, swap one implementation for the other. (I'm not sure this is actually stated in the principle, but I don't think that changes the principle)Loram
It is stated there. OCP only applies to code that is ready for production, ie. it has passed code review and testing. Of course changes to fix eventual bugs found later are allowed. The problem with OCP is that it tells us to prepare the code for future new/modified features through up-front design. In constrast, agile development tells us to solve only what needs to be solved at any given moment, allowing the design to evolve as features are added or modified, without artificial prohibitions; the risk of change is managed with the use of extensive developer testing.Glaikit
You are refering to a methodology here. OCP is not a methodology. It is a principle. By using a methodology (such as Agile) you can reach a design that respects the Open/Closed Principle.Loram
G
3

Alright, so here's my response.

I cannot testify to the historic origin of the principle but it is still invoked frequently in modern times. I don't think its about it being dangerous to change functioning code (though it of course is) its about allowing you to separate out ideas.

Suppose we have a component

public class KnownFriendsFilter{
  IList<People> _friends;
  public KnownFriendsFilter(IList<People> friends) {
    _friends = friends;
  }
  public IList<Person> GetFriends(IList<Person> people) {
    return people.Where(p=>_friends.Contains(p)).ToList();
  }
}

Now say the algorithm on which this specific component needs a slight modification - for example you want to make sure that the initial list passed in contains distinct people. This is something that would be a concern of the KnownFriendsFilter so by all means change the class.

However there is a difference between this class and the feature it supports.

  • This class is really just to filter an array of people for known friends
  • The feature that it supports is to find all friends from an array of people

The difference is that the feature is concerned with function while the class is concerned with implementation. Most requests we get to change that feature will fall outside the specific responsibility of the class.

For example lets say we want to add a blacklist of any names that begin with the letter "X" (because those people are obviously spacemen and not our friends). That's something that supports the feature but is not really a part of what this class is all about, sticking it in the class would be awkward. What about when the next request comes in that now the application is being used exclusively by misogynists and any female names must be also excluded? Now you've got to add logic to decide whether the name is male or female into the class - or at least know about some other class that knows how to do that - the class is growing in responsibilities and becoming very bloated! And what about cross-cutting concerns? Now we want to log whenever we filter an array of people, does that go right in there too?

It would be better to factor out an IFriendsFilter interface and wrap this class in a decorator, or re-implement it as a chain of responsibility on IList. That way you can place each of those responsibilities into their own class that supports just that concern. If you inject dependencies then any code that uses this class (and its centrally used in our application) doesn't have to change at all!

So again, the principle isn't about never changing existing code - it's about not ending up in a situation where you are faced with the decision between bloating the responsibilities of a commonly used class or having to edit every single location that uses it.

Gregarine answered 13/9, 2009 at 0:14 Comment(5)
Well, you seem to be talking about stuff that to me is very different from what OCP talks about. Some of those feature changes would require modifications to the existing class, for example to add a dependency injection point. I think you may be trying to fit a square peg (OCP) into a round hole (making classes more versatile by adding extension points). I don't believe Bertrand Meyer was thinking along the same lines you are. Wouldn't it be better simply to admit that the OCP should be discarded? There certainly are plenty other principles and ideas around...Glaikit
No changes to the class would be needed at all except extract interface if the solution we apply is Decorator. Like I said, I have no historical perspective on OCP but this is what it refers to when used as the O in SOLID. It is ALL about extension points. I urge you to join the next virtual alt.net meeting where incidentally the next presentation will be talking about Open/Closed virtualaltnet.com/Home/CalendarGregarine
Yes, Robert Martin describes OCP as something to be achieved by using abstractions whose implementing classes are chosen through configuration/wiring, while the client classes depend only on the abstractions. Sure, it's a way of doing things, but an ineffective one, compared to the alternative of doing things with simplicity in mind (KISS, YAGNI). Also, that approach disregards the fact the many changes to functionality will require changes to the abstractions anyway.Glaikit
I got into this stuff a little over a year ago so I'm not tremendously experienced. I have however spoken to countless people that have been doing it for much longer. We all disagree with your assertion that changes will usually require you to change the interface. From my experience a smartly placed abstraction can indeed absorb most change requests that do not trigger a mass refactoring. As for a violation of YAGNI - yeah it kind of is. So don't create an interface until you need it. You've got refactoring tools, right? Join the livemeeting and learn about this. PleaseGregarine
I never asserted that changes will usually require you to change the interface; I said that "many changes to functionality will". Your idea about not creating an interface "until you need it" does imply violation of the OCP. The OCP actually requires you to create the interface up-front. See the "Client-Server" example in objectmentor.com/resources/articles/ocp.pdf (page 2).Glaikit
P
2

So, does the OCP really makes sense today? I don't think so.

Sometimes it does:

  • When you've released the base class to customers and can't easily modify it on all your customers' machines (see for example "DLL Hell")

  • When you're a customer, who didn't write the base class yourself and aren't the one maintaining it

  • More generally, any situation where the base class is used by more than one team and/or for more than project

See also Conway's Law.

Pompon answered 13/9, 2009 at 0:3 Comment(4)
I agree with your three points, but the OCP isn't really about modifying published APIs. Most code ever written for an application and also for most reusable class libraries/frameworks is internal, not exposed for an unknown audience. The OCP says than even such code should not be modified, which is what I disagree with.Glaikit
Apparently you can take the principle to absurd extremes: "no software, once written, should ever be changed." There's truth in it though, and even as a solo developer on a single project I find that some of my early (e.g. architecture-level) abstractions are 'stable', and I don't change them as I develop a project (e.g. by adding more implementation details to subclasses).Pompon
Actually it is about published APIs, see the update on my answer.Loram
The most well known OCP article, objectmentor.com/resources/articles/ocp.pdf, says about a module that satisfies the principle (page 2, summarized here): "Open for Extension": this means that we can make the module behave in new and different ways as the requirements of the application change. "Closed for Modification": the source code of such a module is inviolate; no one is allowed to make source code changes to it. So, it's NOT just or even mainly about the published interface of the module. To me, the module source code should not be closed for modification.Glaikit
U
1

Interesting question, and based on the strict definition of the open closed principle I can see where you're coming from.

I have come to define the open-closed principle slightly differently, and this principle I think should apply, and that is to apply it far more broadly.

I like to say, all my classes (as a whole) involved in the application should be closed for modification and open for extension. So the principle is that if I need to change behaviour and/or operation of the application, I do not in fact modify a class but add a new one and then change the relationships to point to this new one (depending of course on the size of the change). If I'm following the single responsibility and utilising inversion of control, this should occur. What then occurs is that all changes come to be extensions. They system now can both act in the former way and in the new way and changing between them = changing a relationship.

Unsound answered 13/9, 2009 at 0:4 Comment(1)
You could do that, but doesn't seem practical to me. There would be many different versions of what is conceptually the same class, and all those separate classes that implement the same interface would coexist in the same codebase, which would grow much more quickly.Glaikit
V
0

The point here is that such modern tools allow a developer to change literally thousands of lines of code, safely, in a few seconds.

Which is fine if you have 'a' developer. If you are working in teams, certainly with version control, probably with branching and merging, then the ability to ensure that changes from different people tend to end up concentrated in different files is pretty vital to being able to control what's going on.

You could imagine language-specific merge/branch tools that could take three refactorings done in parallel and merge them as easily as changes to isolated files. But such tools don't exist, and if they did, I wouldn't want to rely on them.

Venn answered 13/9, 2009 at 0:9 Comment(5)
OK, automated refactorings that actually modify thousands of lines of code don't occur often (even though I have done just that a lot in a team of about 8 developers, using CVS, years ago). But the point is that the tools do allow you to make the changes in a safe way; no branching is required, in my experience, and merging is safe when done correctly even if it is somewhat laborious.Glaikit
Well, everything is safe if done correctly, even juggling chainsaws... The key point is that the only actual change since the 90s has been the increased popularity of branch/merge as opposed to checkin/checkout. Noone was using OCP but passing files around on floppies... And extensive branching actually makes extensive refactoring more, not less, difficult.Venn
Actually, I do refactoring a lot, but never used branches in CVS or SVN. Never needed to. I think you would be amazed to see how maintenance work was performed in my previous project: a large, complex business Java-based web app (400+ persistent entities, 500+ use cases). About a dozen change requests applied every day, for a whole year, with a new version in production every day. There were 7-9 developers, and I wouldn't say they were exceptional. This level of maintenance is still going on, with the app still online. We did not follow OCP.Glaikit
Exactly: because you were not using branches, you had little problem with refactoring. Also, I don't think it is physically possible to program in Java without following OCP at least sometimes. So I suspect you have some kind of misunderstanding of what it is about.Venn
OK, I always suspected branches were more trouble than they were worth. To follow OCP, I would have to create separate abstractions for everything and make all clients only depend on them, not on the implementations. I don't think I ever really did that, or even saw anybody else doing it, in all the projects I participated in. Can you be more specific on what misunderstanding would that be? (If there is one I would really like to know about it; I am trying to be open-minded here, but the arguments so far seem to indicate that people are not really reading about OCP.)Glaikit

© 2022 - 2024 — McMap. All rights reserved.