Open/closed principle (OCP)

The terms open and closed are not something I often hear when discussing software engineering, so perhaps they could do with a little explanation.

Open means that we should be able to extend or adapt code by adding new behaviors and features. Closed means that we should avoid making changes to existing code, changes that could result in bugs or other kinds of regression.

These two characteristics might seem contradictory, but the missing piece of the puzzle is the scope. When talking about being open, we are talking about the design or structure of the software. From this perspective, being open means that it is easy to add new packages, new interfaces, or new implementations of an existing interface.

When we talk about being closed, we are talking about existing code and minimizing the changes we make to it, particularly the APIs that are used by others. This brings us to the first advantage of OCP:

OCP helps reduce the risk of additions and extensions

You can think of OCP as a risk-mitigation strategy. Modifying existing code always has some risk involved, and changes to the code used by others especially so. While we can and should be protecting ourselves from this risk with unit tests, these are restricted to scenarios that we intend and misuses that we can imagine; they will not cover everything our users can come up with.

The following code does not follow the OCP:

func BuildOutput(response http.ResponseWriter, format string, person Person) {
  var err error

  switch format {
  case "csv":
    err = outputCSV(response, person)

  case "json":
    err = outputJSON(response, person)
  }

  if err != nil {
    // output a server error and quit
    response.WriteHeader(http.StatusInternalServerError)
    return
  }

  response.WriteHeader(http.StatusOK)
}

The first hint that something is amiss is the switch statement. It is not hard to imagine a situation where requirements change, and where we might need to add or even remove an output format.

Just how much would have to change if we needed to add another format? See the following:

• We would need to add another case condition to the switch: This method is already 18 lines long; how many more formats do we need to add before we cannot see it all on one screen? In how many other places does this switch statement exist? Will they need to be updated too?
• We would need to write another formatting function: This is one of three changes that are unavoidable
• The caller of the method would have to be updated to use the new format: This is the other unavoidable change
• We would have to add another set of test scenarios to match the new formatting: This is also unavoidable; however, the tests here will likely be longer than just testing the formatting in isolation

What started as a small and simple change is beginning to feel more arduous and risky than we intended.

Let's replace the format input parameter and the switch statement with an abstraction, as shown in the following code:

func BuildOutput(response http.ResponseWriter, formatter PersonFormatter, person Person) {
  err := formatter.Format(response, person)
  if err != nil {
    // output a server error and quit
    response.WriteHeader(http.StatusInternalServerError)
    return
  }

  response.WriteHeader(http.StatusOK)
}

How many changes was it this time? Let's see:

• We need to define another implementation of the PersonFormatter interface
• The caller of the method has to be updated to use the new format
• We have to write test scenarios for the new PersonFormatter

That's much better: we are down to only the three unavoidable changes and we changed nothing in the primary function at all. This shows us the second advantage of OCP:

OCP can help reduce the number of changes needed to add or remove a feature.

Also, if there happens to be a bug in our new structure after adding the new formatter, it can only be in one place—the new code. This is the third advantage of OCP:

OCP narrows the locality of bugs to only the new code and its usage.

Let's look at another example, where we don't end up applying DI:

func GetUserHandlerV1(resp http.ResponseWriter, req *http.Request) {
  // validate inputs
  err := req.ParseForm()
  if err != nil {
    resp.WriteHeader(http.StatusInternalServerError)
    return
  }
  userID, err := strconv.ParseInt(req.Form.Get("UserID"), 10, 64)
  if err != nil {
    resp.WriteHeader(http.StatusPreconditionFailed)
    return
  }

  user := loadUser(userID)
  outputUser(resp, user)
}

func DeleteUserHandlerV1(resp http.ResponseWriter, req *http.Request) {
  // validate inputs
  err := req.ParseForm()
  if err != nil {
    resp.WriteHeader(http.StatusInternalServerError)
    return
  }
  userID, err := strconv.ParseInt(req.Form.Get("UserID"), 10, 64)
  if err != nil {
    resp.WriteHeader(http.StatusPreconditionFailed)
    return
  }

  deleteUser(userID)
}

As you can see, both our HTTP handlers are pulling the data from the form and then converting it into a number. One day, we decide to tighten our input validation and ensure that the number is positive. The likely result? Some pretty nasty shotgun surgery. In this case, however, there is no way around. We made the mess; now we need to clean it up. The fix is hopefully pretty obvious—extracting the repeated logic to one place and then adding the new validation there, as shown in the following code:

func GetUserHandlerV2(resp http.ResponseWriter, req *http.Request) {
  // validate inputs
  err := req.ParseForm()
  if err != nil {
    resp.WriteHeader(http.StatusInternalServerError)
    return
  }
  userID, err := extractUserID(req.Form)
  if err != nil {
    resp.WriteHeader(http.StatusPreconditionFailed)
    return
  }

  user := loadUser(userID)
  outputUser(resp, user)
}

func DeleteUserHandlerV2(resp http.ResponseWriter, req *http.Request) {
  // validate inputs
  err := req.ParseForm()
  if err != nil {
    resp.WriteHeader(http.StatusInternalServerError)
    return
  }
  userID, err := extractUserID(req.Form)
  if err != nil {
    resp.WriteHeader(http.StatusPreconditionFailed)
    return
  }

  deleteUser(userID)
}

Sadly, the original code has not reduced, but it's definitely easier to read. Beyond that, we have future-proofed ourselves against any further changes to the validation of the UserID field.

For both our examples, the key to meeting OCP was to find the correct abstraction.