Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.
Sign up to join this community
I have a class calculating workers' net yearly income. It has a constant representing a tax percentage. But one day the tax rate has changed, so I need to fix the code.
Does the act of fixing this constant indicate a violation of the Open-Closed Principle, since it postulates that a class should be closed to modification?
The OCP can be better understood when thinking of classes or components provided by a vendor A in some kind of black-box library, for usage by users B, C, and D (note this is just a mental model I am using for clarity, it does not matter if in reality the only user of the class is A himself).
If B, C and D can use or reuse the provided classes for different use cases, without the need for modification of the library's source code, then the component fulfills the OCP (in respect to a category of use cases). There are different means to achieve this, like
making the class inheritable (typically in conjunction with the template method pattern or the strategy pattern)
by providing "injection points" for dependency injection
by providing configuration parameters for the class or component (for example, by having a constructor parameter "tax percentage", as in your case, or by using some other configuration mechanism)
maybe other means, depending on the programming language or ecosystem
The typical examples you find in text books are often of the first or second type (I guess because in the eyes of those book's authors, the third type is too trivial to be worth mentioned).
As you see, this has nothing to do with forbidding any change of the source code by vendor A (like for bug fixing, optimization or adding new features in a backwards-compatible manner), that is quite unrelated to the OCP. The OCP is about how A designs the interface and the granularity of the components in the lib, so different reusage scenarios (like resuage with different tax rates) do not automatically induce requirements for change.
So despite what others are told you here, the answer is clearly "yes", it would be a violation of the OCP.
EDIT: seems in between someone wrote a detailed blog post about exact this topic. Though parts of it could have been better worded (as Derek Elkins pointed out), it seems the author generally shares my point of view that "fulfilling the OCP" is no absolut property, but a something which can only be evaluated in context of certain categories of requirement changes.
As others are saying, ideally the worker income class would allow for parameterization of the constant, making this class independent of that value.
Ultimately, the calling application might also allow for parameterization in terms of external configuration (e.g. a file). Once we have external configuration, we can change the tax rate — though consider that if the configuration file is read only once at startup, then the application will have to be restarted for updated tax percentages to take effect, so that is something to keep in mind. We could provide an application feature to reread the configuration when directed to do so, or we might provide a more complicated mechanism that notices when the configuration file changes...
Long term, you may find that the tax issues require more than just a percentage — for example, that one day the tax laws are more complex and requires several percentages and some constants (e.g. the amount under $10k taxed at X%, while the remainder taxed at Y%).
This basically suggests using a strategy pattern, where the main class in question here accepts a strategy object for computing the tax.
The various strategies (and %'s and $ constants) should be choose-able from the configuration file, and now, adding a new strategy requires adding some new code, but not necessarily updates to existing code.
Each strategy might know how to parse/interpret its own external configuration arguments, along with how to compute actual tax.
Dynamically, the tax may further depend on the governing locale, so you might have locale associated with earnings or with employees (or both). In external configuration, we might associate locale with tax strategy.
Also see dependency injection, where we manage these things explictly.
If you need to modify the class to change the tax value, then its design is indeed violating OCP. The appropriate design, for what you've described so far, is for the calculator class to take the tax value as a parameter.
If your class is instanced (meaning it's not a static class), by making the tax variable class property, whose value is injected through the constructor, you'd be also improving the class cohesion.
In short, your current design make your class depend on a constant value that's not really a constant (defining constant as a value that'd never change no matter what, like the value of PI). It violates OCP. Change the design to receive the tax value as constructor argument.
Totally agree with @Becuzz, and I just want to sum this up: OCP is about finding reused (hence, useful) abstractions that are injected in a class. So the behavior of the class is modified not by changing its code, but by providing it with different implementations. This is made crystal clear in Robert Martin's book "Agile Software Development, Principles, Patterns, and Practices", check the corresponding chapter "The Open-closed principle", "Abstraction is the Key" sub-chapter. It clarifies another misconception that behavior can be modified only with inheritance. It was Bertrand Meyer who proposed that in 1988 in his book “Object Oriented Software Construction”, not Robert Martin.
The way I see it is not a violation of the open closed principle. Yet, the fact that something that is bound to change in time (such as the tax percentage) is a constant is a design flaw: you should not change the value of the constant but how you handle the tax percentage. This should be some type of setting that could be modified without recompiling the whole thing.
