8

I am bit confused about how Open Closed principle can be applied in real life. Requirement in any business changes over the time. According to Open-Closed principle you should extend the class instead modifying the existing class. To me every time extending a class does not seem to be practical to fulfill the requirement. Let me give an example with train booking system.

In train booking system there will be a Ticket object.There could be different types of tickets Regular Ticket, Concession Ticket etc. Ticket is abstract class and RegularTicket and ConcessionTickets are concrete classes. Since all the tickets will have PrintTicket method which is common, hence it is written in Ticket which is base abstract class. Let's say this worked fine for few months. Now new requirement comes in, which states to change the format of the ticket. May be few more fields are added on the printed ticket or may format is changed. To fulfill this requirement I have following options

  1. Modify the PrintTicket() method in Ticket abstract class. But this will violate Open-Closed principle.
  2. Override the PrintTicket() method in child classes but this will duplicate the printing logic which is violation of DRY(Do not repeat yourself) Principle.

So questions are

  1. How can I satisfy above business requirement without violating the Open/Closed principle.
  2. When the class is supposed to be closed for modification? What are the criteria to consider class is closed for modification? Is it after initial implementation of the class or may be after first deployment in production or may be something else.

migrated from stackoverflow.com May 19 '16 at 19:08

This question came from our site for professional and enthusiast programmers.

  • 1
    I guess that the problem here is that your ticket class should not know how to print itself. A TicketPrinter class should receive a ticket and know how to print it. In this case you can use the decotator patern to increment you printer class or override it to change it completly. – Vinicius Zaramella Apr 14 '16 at 14:36
  • 1
    Decorator will also face the same Open/Closed problem. Requirement may change many times in future. In that case will you change the decorator or extend the decorator. Extending decorator every time does not seem to be practical and modifying decorator will violate the open/closed principle. – parag Apr 14 '16 at 14:42
  • 1
    It is very hard to design with OCP for arbitrary changes in design. It's a gamble to pick the dimension you expect to vary with respect to the part that is stable. – Fuhrmanator Apr 16 '16 at 15:25
5

I have following options

  • Modify the PrintTicket() method in Ticket abstract class. But this will violate Open-Closed principle.
  • Override the PrintTicket() method in child classes but this will duplicate the printing logic which is violation of DRY(Do not repeat yourself) principle.

This depends on the way the PrintTicket is implemented. If the method needs to consider information from subclasses, it needs to provide a way for them to supply additional information.

Moreover, you can override without repeating your code too. For example, if you call your base class method from the implementation, you avoid repetition:

class ConcessionTicket : Ticket {
    public override string PrintTicket() {
        return $"{base.PrintTicket()} (concession)"
    }
}

How can I satisfy above business requirement without violating the Open/Closed principle?

Template Method pattern supplies a third option: implement PrintTicket in the base class, and rely on derived classes to supply additional details as needed.

Here is an example using your class hierarchy:

abstract class Ticket {
    public string Name {get;}
    public string Train {get;}
    protected virtual string AdditionalDetails() {
        return "";
    }
    public string PrintTicket() {
        return $"{Name} : {Train}{AdditionalDetails()}";
    }
}

class RegularTicket : Ticket {
    ... // Uses the default implementation of AdditionalDetails()
}


class ConcessionTicket : Ticket {
    protected override string AdditionalDetails() {
        return " (concession)";
    }
}

What are the criteria to consider class is closed for modification?

It's not the class that should be closed to modification, but rather the interface of that class (I mean "interface" in the broad sense, i.e. a collection of methods and properties and their behavior, not the language construct). The class hides its implementation, so the owners of the class can modify it at any time, as long as its externally visible behavior remains unchanged.

Is it after initial implementation of the class or may be after first deployment in production or may be something else?

The interface of the class needs to remain closed after the first time that you publish it for external use. Internal-use classes remain open to refactoring forever, because you can find and fix all its usages.

Apart from the simplest cases, it is not practical to expect that your class hierarchy is going to cover all possible usage scenarios after a certain number of refactoring iterations. Additional requirements calling for entirely new methods in the base class come up regularly, so your class stays open to modifications by you forever.

  • Yes, In this particular case templating will work. But this is just an example to put the problem. There could be many scenarios/business problems where templating/plugin method may not be applicable. – parag Apr 14 '16 at 14:31
  • 1
    Your point on the class interface is in my opinion the best resolution to this particular scenario. To focus it in on the specific question: one could have a PrintTicket method with a specified output defined in the interface for the Ticket class (along with defined inputs) then as long as the ticket output is always the same it would not matter how the underlying classes changed. - Also if the properties of a ticket are always expected to change, why not design your class with that as a known business requirement. Ticket should contain a collection of a ticketProperty object of some sort – user3908435 Apr 14 '16 at 15:13
  • Could also make a TicketPrinter class that's passed in to the constructor. – Zymus May 19 '16 at 19:59
2

Let's start with simple line of Open-closed principle - "Open for extension Closed for modification". Consider your problem I would design the classes so that base Ticket would be responsible for printing a common Ticket information and other Ticket types would be responsible for printing their own format on top of base printing.

abstract class Ticket
{
    public string Print()
    {
        // Print base properties here. and return 
        return PrintBasicInfo() + AddionalPrint();
    }

    protected abstract string AddionalPrint();

    private string PrintBasicInfo()
    {
        // print base ticket info
    }
}

class ConcessionTicket : Ticket
{
    protected override string AddionalPrint()
    {
        // Specific to Conecession ticket printing
    }
}

class RegularTicket : Ticket
{
    protected override string AddionalPrint()
    {
        // Specific to Regular ticket printing
    }
}

This way you're enforcing any new type of ticket that would be introduced in the system will implement it's own printing feature on top of basic ticket printing information. Base Ticket is now closed for modification but open for extension by providing additional method for it's derived types.

2

The problem is not that you are violating the Open/Closed principle, the problem is that you are violating the Single Responsibility Principle.

This is literally a school book example of an SRP problem, or as wikipedia states:

As an example, consider a module that compiles and prints a report. Imagine such a module can be changed for two reasons. First, the content of the report could change. Second, the format of the report could change. These two things change for very different causes; one substantive, and one cosmetic. The single responsibility principle says that these two aspects of the problem are really two separate responsibilities, and should therefore be in separate classes or modules. It would be a bad design to couple two things that change for different reasons at different times.

The different Ticket classes may change because you add new information to them. The printing of the ticket may change because you need a new layout, hence you should have a TicketPrinter class that accepts tickets as input.

Your Ticket classes can then implement different interfaces to provide different types data or provide some kind of data template.

1

Modify the PrintTicket() method in Ticket abstract class. But this will violate Open-Closed principle.

This does not violate the open-closed principal. Code changes all the time, that's why SOLID is important, so code remains maintainable, flexible, changeable. There's nothing wrong with changing code.


OCP is more about external classes not being able to tamper with a class's intended functionality.

For instance, I have a class A that takes a String in the constructor, and verifies this String has a certain format by calling a method to verify the string. This verifying method also needs to be usable by clients of A, so in the naive implementation, it's made public.

Now I can 'break' this class by inheriting from it and overriding the verifying method to just always return true. Due to polymorphism, I can still use my subclass anywhere where I can use A, possibly creating unwanted behaviour in my program.

This seems like quiet a malicious thing to do, but in a more complex code base, it might be done as an 'honest mistake'. To conform to OCP, class A should be designed in such a way that making this mistake is not possible.

I could do this by, for instance, making the verifying method final.

  • I've never seen a description of OCP that would state that this is a violation of it. LSP, definitely, but not OCP. – Jules May 19 '16 at 23:40
  • @Jules I found later that the wikipedia definition is indeed very different from this. This was one of the examples used to teach me OCP though (though it might have been slightly different). The point is, it wasn't about forbidding yourself from changing your code. It doesn't make sense to me like that either. If you write code, and the environment changes so that your code doesn't fit any more, change it, or better, throw it away and start again. Why keep something that's broken... – Jorn Vernee May 19 '16 at 23:58
0

Inheritance isn't the only mechanism that can be used to fulfil the requirements of OCP, and in most cases I would argue that it is rarely the best -- there are usually better ways, as long as you plan ahead a little to consider the kind of changes you're likely to need.

In this instance, I would argue that if you expect to get frequent format changes to your ticket printing system (and that seems like a fairly safe bet to me), using a templating system would make a lot of sense. Now, we don't need to touch the code at all: all we have to do to fulfil this request is change a template (as long as your template system can access all the data that's required, anyway).

In reality, a system can never be entirely closed against modification, and even getting it close would require a huge amount of wasted effort, so you have to make judgment calls over what kind of changes are likely, and structure your code accordingly.

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.