Do we need to adhere to the Open-Closed Principle if we have good test coverage?

Question

Why Open-Closed Principle is such a big deal? If you have tests why you should worry about code modifications?

Answer 1

If you have tests why you should worry about code modifications?

In the context of unit testing, the open-closed principle is a design principle that allows you to add new code with minimal impact on code that has already been written before hand. What has been tested will not be changed because of new requirements, thus reducing the area of damage a programmer can create over a stable code base. Consider the following pseudo object-oriented code example that does not follow the open-closed principle :

class NumberOperations {
   public void add(Number a,Number b) {
      Number result = null;
      if(a instanceof Integer && b instanceof Integer) {
         Integer aInt = (Integer)a;
         Integer bInt = (Integer)a;
         result = aInt + bInt;
      } else {
          //failure/error : only integer operations supported.
      }
      return result;
   }
}

You test the above code and all works as expected. You now decide to support decimal operations as well so you modify the add function as follows :

public void add(Number a,Number b) {
    Number result = null;
    if(a instanceof Integer && b instanceof Integer) {
        Integer aInt = (Integer)a;
        Integer bInt = (Integer)a;
        result = aInt + bInt;
    } else if(a instanceof Double && b instanceof Double) {
        Double aDecimal = (Double)a;
        Double bDecimal = (Double)b;
        result = aDecimal + bDecimal ;
    } else {
      //failure/error : only integer and double operations supported.
    }
    return result;
}

With just one new simple requirement of supporting decimal addition, you have added 4 new lines of code, rearranged one line of code and doubled your area of damage. This may look trivial with such a simple example but real world code is seldom this basic. You may introduce side-effects to existing code that none of the test cases can fathom.

If you adhered to the open-closed principle, you could have subclasses called IntegerOperations and DoubleOperations extending from NumberOperations (which could now be abstract) that implement the add method. The code inside the first if in the add method shown above could be moved to the add method in IntegerOperations class and the code in the else-if could be moved to the add method in DoubleOperations class. If your language supports generics, you could even get rid of the type checks that the above example shows.

You then test out these two new classes individually. When you get a requirement to add two Strings, you create a new class called StringOperations. You don't have to worry about any unwanted changes being made to IntegerOperations or DoubleOperations and therefore don't have to test them again. You only need to test the StringOperations class.

In Summary : OCP not only reduces the area of damage that you may expose your codebase to but also allows users of your API to add new functionality (via subclassing for example) without the need to rely on you for making these changes. You may be able to ensure all critical code paths are tested without OCP. What you can't protect your codebase from is the risk of someone else breaking code that has already been tested which introduces a bug you have not even accounted for in your tests.

Answer 2

In a perfect world you would have tests which cover 100% of the code base and you could also measure that by using tools. However, neither of those are true. Getting 100% coverage and making sure that 100% coverage still works when the application is five years old is very hard. You might not have the time required to achieve it before a release. You have some other persons on the team that isn't as strict. You might simply have a deadline that must be honored. Tools that show 100% typically understands that a code line have been reached by one execution branch while on another where it also can be reached have not been executed.

In a perfect world your code is also readable and easy to understand. In a real world the readability is not just dependent on you, but your entire team. It's also dependent on if you really can choose the best solution of every problem you get during the entire applications lifetime. I got 20 years of experience and I still find myself write shitty code now and then. The reason is that I'm not perfect and I do not always have perfect conditions like crystal clear requirements or a great description of what the customer wants.

The SOLID principles and other best practices and principles help you since the world is not perfect. But by following principles like Open/Closed principle you give your future self a much better chance of dealing with your code base. Those principles have been defined by experienced people who have learned the hard way what works and what not.

Answer 3

Of course if you have really really good system tests you can be confident your code still works even with big, principle-breaking edits, so despite what some principle fanatics would have you think, the principle isn't a necessity - but it is a practicality. You don't have to use it, but you'll generally make things easier for yourself if you do. The helpful practicality with the O/C principle is being able to extend existing code to meet new requirements without risking breaking existing uses of that code. Even if you don't end up needing to spend time fixing existing code because the edits didn't break it, you'd still need to retest it.

When considering the existing tests (and I feel this applies more to unit tests here than system tests) you've got to consider whether edits that go against the closed part of the o/c principle will break or invalidate existing tests. If you break existing tests with a change of interface or a change of required pre-condition you can't truly use the same tests on both the old and new implementations (your new tests may be based on the old but wont be exactly the same). Perhaps more worrying (just because it won't make itself obvious like broken tests will) is that the edits may mean that there should be some additional tests that aren't already there - maybe you've introduced new special cases. The edits might have unintended side effects that are not tested for.

Ultimately the O/C principle has to just be an aim, with some principles we can tell for sure whether we have successfully followed them. With the O/C principle we might discover that when we actually want to extend it, the original is not as open as we'd hoped, and if it turns out the original didn't work properly its going to need changing even if you think this goes against the closedness.

Like many principles it's not that it's essential it's that in general it's helpful and the effort to follow it will pay off over time.