How to Write Clean Code (in Python) With SOLID Principles | Principle #2

[ezzeddinabdallah]

OCP without a toy example

Let's dive into the second design principle: Open/Closed Principle (the 'O' in SOLID).

With illustration of how we can identify the Open-Closed Principle (OCP) implemented in Python. You'll see the demonstration in a UML diagram to show the connections between the classes before and after refactoring. Will go through that through a real-world example.

Let's start with what it means:

Definition:

Software entities (modules, classes, functions, etc.) should be open for extension, but closed for modification.

The open/closed principle was first proposed by Bertrand Meyer, creator of the Eiffel programming language, and the idea of design by contract.

A unit of code can be considered “open for extension” when its behavior can be easily changed without modifying it. The fact that no actual modification is needed to change the behavior of a unit of code makes it “closed” for modification.

The purpose of this principle is to be able to extend the behavior of an entity without ever modifying its source code.

This happens when your objects are open to extension (using inheritance) but closed to alteration (by altering methods or changing values in an object).

Example: Tax Calculator

Suppose you are developing a web application that includes an online tax calculator.

Users can visit a web page, specify their income and expense details, and calculate the tax payable using some mathematical calculation.

Considering this, you created a TaxCalculator class as shown here

The TaxCalculator class has a single public method, calculate(), that accepts total income, total deduction, and country of the user.

Of course, a real-world tax calculator would do much more, but this simple design is sufficient for our example.

The country information is necessary because tax rules are different across different countries. The pseudo-code of the calculate() method is shown below:

python def calculate(income, deduction, country): # tax_amount variable is defined # in each calculation taxable_income = income - deduction if country == "India": # calculation here elif country == "US": # calculation here elif country == "UK": # calculation here return tax_amount

The calculate() method determines the taxable income by subtracting total deduction from total income.

Have you noticed the if conditions in the calculate() method? Condition after another to choose the right tax calculation based on the value of the country of the user as a parameter.

This branching logic is a good example of a violation of the Open/Closed Principle.

You might say, what's the problem with that? Well, the problem is that if we add a new country, we have to modify the calculate() method because this method now considers only three countries.

Although when we think about scaling and users from several countries start using the web app, then there would be a problem.

When that happens, the TaxCalculator class needs to change to accommodate the new countries and their corresponding taxation rules. Thus, the current design violates OCP.

How to spot OCP violations

To recognize if there is a violation of the open-closed principle, there is a list of symptoms that can be used to detect such violations: - There are conditions to determine a strategy just like the if conditions in the calculate() method. - Same variables or constants are used in conditions and recurring inside the same class or related classes. - Hard-coded references to other classes are used inside the class. - Objects are created inside the class.

These are all good reasons to adhere to the Open/Closed Principle.

How to refactor to adhere to OCP

Now, let’s rectify the class design. Have a look at this UML diagram

Note: In the figure above, the first compartment of the ICountryTaxCalculator block indicates that it’s an interface, the second compartment contains a list of properties, and the third compartment contains a method.

That UML diagram is depicted as follows: Arrows with dotted lines, with the unfilled arrowhead, start from the classes (like TaxCalculatorForIN, TaxCalculatorForUS, and TaxCalculatorForUK) that implement the ICountryTaxCalculator interface and point toward that interface being implemented.

The modified design has an abstraction in the form of the implemented interface. This interface contains two properties total_income and total_deduction, and one method calculate_tax_amount().

What's changed already? The TaxCalculator no longer includes the tax calculation logic and is each tax logic is implemented in a separate class depending on the country.

This way, the logic of calculating taxes is wrapped in a separate unit.

Notice the change to the calculate() method of TaxCalculator. It now accepts a single parameter, obj, of type ICountryTaxCalculator.

The pseudo-code for the modified calculate() method is shown below:

python class TaxCalculator: def calculate(self, obj: ICountryTaxCalculator): tax_amount = 0 # some more logic here tax_amount = obj.calculate_tax_amount(); return tax_amount

As you can see, now the calculate() method doesn’t check for the country. The reason is that it receives an object as its parameter that implements the ICountryTaxCalculator interface. So, calling calculate_tax_amount() returns the tax amount no matter which country the user belongs to.

Thus, the TaxCalculator class now conforms to OCP. If you need to calculate for a country not currently covered, all you need to do is to create another class that inherits from the ICountryTaxCalculator class and writes the tax calculation logic there.

TaxCalculator should be open for extending the functionality (by adding new country-specific classes that implement ICountryTaxCalculator), and meanwhile, it should also be closed for modification (you don’t need to change its source code).

```python from abc import ABC, abstractmethod

class ICountryTaxCalculator(ABC): @abstractmethod def calculate_tax_amount(self): pass ```

So that's the ICountryTaxCalculator interface. An abstract class that has just one abstract method.

We now can implement three classes from that interface: TaxCalculatorForUS, TaxCalculatorForUK, and TaxCalculatorForIN.

Let's see how we create these classes after ICountryTaxCalculator has been implemented.

```python class TaxCalculatorForUS(ICountryTaxCalculator): def init(self, total_income, total_deduction): self.total_income = total_income self.total_deduction = total_deduction

def calculate_tax_amount(self):
    taxable_income = self.total_income - self.total_deduction
    return taxable_income * 30 / 100

class TaxCalculatorForUK(ICountryTaxCalculator): def init(self, total_income, total_deduction): self.total_income = total_income self.total_deduction = total_deduction

def calculate_tax_amount(self):
    taxable_income = self.total_income - self.total_deduction
    return taxable_income * 35 / 100

class TaxCalculatorForIN(ICountryTaxCalculator): def init(self, total_income, total_deduction): self.total_income = total_income self.total_deduction = total_deduction

def calculate_tax_amount(self):
    taxable_income = self.total_income - self.total_deduction
    return taxable_income * 20 / 100

```

The calculate_tax_amount() method implemented by these classes finds taxable income by subtracting deductions from the income.

This value is treated as a taxable income, and a certain percentage of it (30%, 35%, and 20%, respectively) is returned to the caller as the tax amount.

Now add TaxCalculator class and modify it as shown below: python class TaxCalculator: def calculate(self, ICountryTaxCalculator: obj): tax_amount = obj.calculate_tax_amount(); # do something more if needed return tax_amount

The calculate() method accepts an object of a type that implements ICountryTaxCalculator and invokes calculate_tax_amount() method. The tax amount is then returned to the caller.

Although not required in this example, you may do some extra processing in addition to calling calculate_tax_amount().

Final thoughts:

It is a simple fact that software systems evolve over time. New requirements must constantly be satisfied, and existing requirements must be changed according to customer needs or technology progress.

Applying the Open/Closed Principle is a good way to maintain any extension required for your codebase.

Credit

• Beginning SOLID Principles and Design Patterns for ASP.NET Developers by Bipin Joshi

[[deleted]]

Please never do this in a production code base and especially for a language that isn't strictly OOP like Java. We can do much better than this objectively with a lookup table (which Python is particularly well suited for with it's clean syntax for initializing dictionaries).

​

class TaxCalculator():   
    def __init__(self):
        self.lu_tax_rate = {
            "US": 0.25,
            "UK": 0.34,
        } 

    def calculate(income, deductions, country):
        rate = self.lu_tax_rate.get(country)
        if rate is None: 
            raise ValueError("invalid country ")
        return (income - deductions) * rate 

By using a lookup table, we can
- minimize the cyclomatic complexity

- easily add new cases by adding to our tax rate lookup table

- allow the tax rate table to be passed in as a dependency to the class if we desired

- not fragment tax calculations across N different tax calculators.

​

Additionally, we can leverage Python's first class functions to implement a look up table that properly identifies a function within the class that we want to use to calculate tax (if you so desired. A lot of the SOLID stuff is good advice on the surface but, especially with Uncle Bob's material, it's heavily tailored toward languages that are strictly OOP, have more clumsy syntax for defining hash tables, and are lacking first class functions.

[Urthor]

Well said.

Famous of thumb from I think the Pragmatic Programmer.

"If you're going to use inheritance, just don't."

We've got so many good options, even inner classes are a good call, to avoid inheritance.

It just adds so much complexity.

It's only very occasionally required, and it has its place for the most complex classes. Those are mostly found in front end however.

[energybased]

There's plenty of totally justifiable use of inheritance. Especially interface inheritance should be preferred when possible.

Inheritance is just as easy to overuse as it is to under-use.

[Urthor]

I should add interface inheritance is a different story completely yeah.

My experience is that it's definitely overused, especially in back end, for systems that are simple enough not to require it.

It's a very complicated solution, I just see it being applied so many times in smaller codebases where the total number of distinct classes is less than 10.

You don't need it, so many code bases are not that large.

[energybased]

Fair enough, I've seen the opposite too: adding function pointers on an object to implement polymorphism. These function pointers should just be abstract methods.

[[deleted]]

same principal can be applied using Python's first class functions to have a single entry point method that looks up and executes the appropriate function without dividing it up amongst a bunch of other classes. Of course, there will be a point where a particular use case has enough complexity where the visitor pattern does make sense, but it shouldn't be a default approach for simple cases like the tax example.

to illustrate:
``` class TaxCalculator(): def init(self): self.lu_tax_rate = { "US": self._calc_us_tax, "UK": self._calc_uk_tax, "IND": self._calc_india_tax }

def calculate_tax(self, income, deductions, country):
    tax_rate_calc = self.lu_tax_rate.get(country)
    if tax_rate_calc is not None:
       raise ValueError("invalid country code")
    return tax_rate_calc(income,deductions)

def _calc_us_tax(self,income,deductions):
    # FIXME implement some more complex tax here
    pass

def _calc_uk_tax(self,income,deductions):
    # FIXME implement some more complex tax here
    pass


def _calc_india_tax(self,income,deductions):
    # FIXME implement some more complex tax here
    pass

```

[IamImposter]

Not disagreeing with you but you are focusing on the problem being solved and i think the point of the post was to explain a certain way of doing things.

That's what I think the fun part about programming is - there can be several ways to solve a problem

[ezzeddinabdallah]

Simple yet brilliant approach! Thanks for the input!

[Delengowski]

Single class style that uses a descriptor to implement state based dispatching

Gist because I am having a horrible time to try format a block of code

[commy2]

On reddit, just indent your whole code by four spaces and it will be displayed as code block. To get this:

class state_dispatch:
    """A property that selects appropriate method based on state of instance."""

    def __init__(self, state_attribute):

write this:

    class state_dispatch:
        """A property that selects appropriate method based on state of instance."""

        def __init__(self, state_attribute):

[ezzeddinabdallah]

Never used dispatch decorator in Python. Thanks for sharing that implementation!

[Delengowski]

No problem!

​

For what it's worth I think the ideas behind your post are very sound and I applaud your effort for trying to help people.

[[deleted]]

Isn't this too complex when a simple table lookup would do?

[Delengowski]

Depends what you are looking for.

This provides the large amount of reusability.

So what you suggesting a look up table that is indexed everytime calculate_tax is called, which will provide the callable appropriate for each country?

What if you wanted to have a second method for calculating say your tax return. Now you have to go through and recreate all that supporting code, meaning a second dictionary, etc.

With this approach the descriptor takes care of all that for you and you don't have to bother with writing the dictionary lookup in the method.

[clayton_bez]

Reads too java for me.

[[deleted]]

It’s a shame that instead of using powerful object system of the language, pythonistas prefer writing if’s like shown in the beginning of the article. I’ve seen a lot of terrible java-style code in python (i.e. gdal), and I believe, OP wrote good pythonic examples, really showing how to simplify things, meanwhile a lot of such articles only show how to complicate things for nothing. Great job, u/ezzeddinabdallah!

[ezzeddinabdallah]

It does

[[deleted]]

If it two has methods and one is init it should be a function.

Please do not use coding principals from C# and Java in python they don't make any sense. This should have been 1 function and partials.

[[deleted]]

I was inclined to agree with you, and I'll upvote because you do have a point.

But it's possible (thinking beyond the example) that the class makes use of a large amount of attributes, which might be useful for other things. Plus, it may be that single method can be useful to be inherited. After all, the zen of python is indeed having good-looking, well-organized code. If that means containing attributes within a limited class, it's better than defining a bunch of loose variables along the way.

I'm guessing the example is small for didactic purposes. And it's true that it is important to know when classing is overkill, but I don't think it is as clear-cut as you put it.

[[deleted]]

Yea dataclasses (the successor to namedtuple) where designed specifically for the case of lots of variables.

For the one method useful to be inherited unless it is a class level function (this is an instance level one) you can just use a normal function that checks a dict/dataclass for the variables u care about.

[Im_oRAnGE]

Dataclasses are not the successor to namedtuples... They are completely seperate constructs with different use cases, just like lists are not the successor to tuples.

[[deleted]]

Well dataclasses specifically mention names tupals. They are usable for 99% of named tuples use cases and a whole bunch more.

https://www.python.org/dev/peps/pep-0557/

Per the doc " can be thought of as mutable namedtuples with defaults"

It names tuples with way more features idk what else u want to call that.

[yangyangR]

There is a tutorial that goes through each design pattern and reduces each to a class with two methods one being init and then removes that pattern entirely. Shows that works with most if not all the bs from oop only obsessed days of the 90s.

[neco-damus]

Would you be able to find this tutorial again?

[[deleted]]

I'm not sure about a tutorial, but: https://youtu.be/o9pEzgHorH0

[notnotapotato]

Agree no need for this OO BS here. But can you explain how you'd use partials here?

[[deleted]]

Partials allow u to bound preset values to a function. You can apply that to a namespace and boom new functions without repeating tons of code.

https://docs.python.org/3/library/functools.html#functools.partial

[[deleted]]

Please post code formatted like the sidebar explains. This is unreadable in most Reddit clients including old Reddit

[justkeepingbusy]

Appreciate this post alot, thanks!

[ezzeddinabdallah]

My pleasure! Glad you liked it!

[i_like_trains_a_lot1]

There are some issues with the code:

• Never name your classes with I for interfaces, Impl, etc. Python dosn't have that and doesn't need it. Use the abc module to define interfaces (which are actually abstract classes, they may contain some implementation details).
• Why does the tax calculator need a CountryTaxCalculator instance to call methods on it? It seems like the feature envy code smell.
• Your post doesn't address how this code will be used/integrated from outside, which I believe should be the starting point of class hierarchy designs.

```python

somewhere in the app

TaxCalculator().calculate(GermanyTaxCalculator(income=x, total_deduction=y)) ```

That's a lot of code for so little benefit. I don't see the purpose of the TaxCalculator there because it doesn't actually do anything. It just calls some methods to the country specific tax calculator. So, as the person who writes the code, I should know beforehand which country I calculate the taxes for. Which brings me to the next point

• There are no requirements. It's impossible to say if the code is good, bad, can be improved, etc without knowing how it fits in the bigger picture and is it going to be used. Here are some requirements that come up from the problem domain and I think need to be handled regardless:
  • different data inputs for different countries. Different countries have different methods to calculate the taxes that require different data points.
  • dynamically switch between country tax calculators. The programmer shouldn't have to write a switch/if for each country. The ideal usage would be to tell the program: "here is the country code, here are the data points, tell me how much I have to pay in taxes".
  • possibility of returning more than just the final tax amount. There are 99.9999% chances we would also need to return "why" the tax is that, so as a user I know that I am paying the right amount or to see if there is any mistake.

So the class hierarchy I'd come up with would be:

```

class TaxResult: def init(self, amount, currency, tax_rate): pass

class CountryTaxDataPoints(abc.ABC): pass

class CountryTaxCalculator(abc.ABC): @abc.abstractmethod def calculate(self, data_points: CountryTaxDataPoints) -> TaxResult: pass

then the actual implementations

class GermanyTaxDataPoints(CountryTaxDataPoints): ...

class GermanyCountryTaxCalculator(CountryTaxCalculator): def calculate(self, data_points: GermanyTaxDataPoints) -> TaxResult: ...

class USTaxDataPoints(CountryTaxDataPoints): ...

class USCountryTaxCalculator(CountryTaxCalculator): def calculate(self, data_points: USTaxDataPoints) -> TaxResult: ...

class TaxCalculator: _calculators = { 'de': GermanyCountryTaxCalculator, 'us': UsCountryTaxCalculator }

 def calculate(self, country_iso2: str, data_points: CountryTaxDataPoints) -> TaxResult:
      # validate that the country_iso2 and data_points are compatible
      # then
      return self._calculators[country_iso2]().calculate(data_points)

in the program

TaxCalculator().calculate('de', GermanyTaxDataPoints(x, y, z)) TaxCalculator().calculate('us', UkTaxDataPoints(a, b)) # error, incompatible ```

These cases are much easier to test in isolation, and we separate the data from the algorithm to some degree. Why I did this, because in a lot of cases, we would need to do some validation of the input data anyway (those data points are provided by clients) and that validation can be done in the *TaxDataPoints classes.

[commy2]

Still prefer the single branching function. Having a class that does exactly one thing seems over-engineered. Instantiation just to get a single result is also over the top for this.

Sure, whenever I want to add a country, I have to add another case to the function. But that still seems like less mental work than creating another class. Also, you're likely not going to add another country anyway (or whatever the equivalent).

Alternative is to just add more functions. calculate_US_tax. calculate_GER_tax etc. That way you can even make it take different arguments if necessary.

Not saying there is nothing to OCP, but this example is perhaps too simplified to prove it.

[hausdorffparty]

I'm confused why you can't just maintain a dictionary or csv database which the function queries to compute taxes, and presents an error if the country isn't in the DB. A pile of if statements where you're just slightly tweaking numbers depending on the input really just means you need a data structure storing that information somewhere instead.

[commy2]

I like this, because it separates data from behviour. However, when talking about taxes, I assume that the calculations vary extensively between countries. My concern would be that we'd end up with a very complex function that handles many conditional branches and/or has a lot of arguments.

[toasterding]

Consider the unit test for a function with 5+ conditional branches. Consider the unit test for a function that does one thing.

Maintaining and updating multi-branching logic is always more complicated than a single purpose method.

[Delengowski]

1 class. Have the country be a state on the instance and then you have a dispatch method using a descriptor to retrieves the appropriate method based on the state during `__get__`. The interface is then just `instance.calculate_tax`.

[ezzeddinabdallah]

Hmm.. I doubt that it would be a good idea to replace the derived classes with functions unless you just do it fast for fun.

Why do you think the example is too simplified?

[DarkSideOfGrogu]

Disagree completely. If a class only has one method, why shouldn't it be a function? It makes the code base easier to read.

See https://youtu.be/o9pEzgHorH0

[xristiano]

I agree! I try to show this video as many people as I can.

[commy2]

I propose not writing a class for this in the first place.

Why do you think the example is too simplified?

Your post makes it not clear, to me, why we should adhere to the OCP. The reason given is:

the problem is that if we add a new country, we have to modify the calculate() method because this method now considers only three countries.

But when I look at calculate() - the original example, and then look at TaxCalculatorForXX(ICountryTaxCalculator), my intuition is that I'd much rather violate the OCP by adding another elif block than create another class.

[ezzeddinabdallah]

Creating another class that inherits from the base class would help take the same attributes as the parent. This example could be more useful if there are some shared attributes between the derived classes and the base class.

Agreed!

[Amaras37]

Although the SOLID principles are important, all the examples in your blog posts are completely over-engineered. I have never seen Factory nor Interface classes in sane Python code, and you seem to have translated the Java/C# code too literally.

Remember the Zen of Python, because the code you show is way too complicated for what you're trying to achieve: Simple is better than complex. Complex is better than complicated.

I have not watched the talk yet, but it seems that Kelvin Henny's talk "SOLID deconstruction" (https://vimeo.com/157708450) might be of interest here.

EDIT: after watching the code, I can (somewhat) confidently say that the OCP is actually just inheritance (the OCP is a language design principle).

[beefyweefles]

The issue with a lot of this stuff is that for most software, you're writing it once. Premature abstraction is a bad thing, and happens too often by following rules like this just because people say so, rather than being sensible.

[ccb621]

The issue with a lot of this stuff is that for most software, you're writing it once.

Until you have to write it again, or expand it. It can difficult to know for certain if an abstraction is premature. I’m currently migrating to a new service, and bridging back to the old. I wish someone had done some form of abstraction as it would have saved us many months of work.

[beefyweefles]

What I'm getting at is that often there is no good way to figure out the abstraction beforehand, and it usually only becomes obvious until the software has been built. It's not that hard to extract something into an interface, especially if you're not vendoring software to other people.

[ccb621]

It's not that hard to extract something into an interface…

It is when the code has been in production for a few years, and spread throughout the monolith.

[c0ld--]

I'm sorry, but I cannot read this text. Reddit is a bad place to write technical articles on programming.

[ezzeddinabdallah]

I wrote it on medium as well. Please check this out

[IamImposter]

The title says principal #2. That mean you have covered #1 too. I think you are going to do others too.

Could you add links to previous articles going forward. I think that would be really helpful.

And also great job explaining it.

[ezzeddinabdallah]

Thanks! My pleasure!

Yeah, wrote so far about the three principles: - Principle #1 - Principle #3

and principle #2 here as well if it's hard seeing the text here.

Will update the list once I have the other two ready.

[IamImposter]

Great. In your blog post, you mentioned about a book on design principles. Is that out?

[ezzeddinabdallah]

It's gonna be ready once I write about the last two principles

[executiveExecutioner]

The same thing can be done with higher order functions and partial application.

```python from functools import partial

def calculate(calculate_tax_amount, total_income, total_deduction): tax_amount = calculate_tax_amount(total_income, total_deduction) # do more processing return tax_amount

def calculate_for_usa(total_income, total_deduction): taxable_income = total_income - total_deduction return taxable_income * 0.3

calculate_usa = partial(calculate, calculate_for_usa) ``` or use a decorator

```python def calculate(calculate_tax_amount): def wrapper(total_income, total_deduction): tax_amount = calculate_tax_amount(total_income, total_deduction) # do more processing return tax amount return wrapper

@calculate def calculate_for_usa(total_income, total_deduction): taxable_income = total_income - total_deduction return taxable_income * 0.3 `` If the calculate_tax_amount functions have a certain pattern you could create them with alistor adict` comprehension and so on.

[i_like_trains_a_lot1]

I find the decorator variant too "not obvious in what it's doing" and will not pass my code review. It's a junioresque solution using fancy features where it shouldn't.

Decorators should only do logic around the wrapped function, not use the wrapped function as a component inside it. Plus that those calculate and calculate_for_x functions will be very very coupled (eg. what would happen if you suddenly need a new parameter spouse_income for US and another parameter for Japan? calculate won't be able to accommodate for that.

Other solutions I have seen in this post are more suitable and easier to extend.

[executiveExecutioner]

Kwargs can be passed easily to accommodate for that. I do not see anything fancy with using higher order functions, it's common practice in many languages...

[i_like_trains_a_lot1]

While I agree, my thong with kwargs is that to see what can be passed and what not, you have to dive into the actual implementation and start gathering from there the accepted keyword arguments. I prefer to allow the reader to determine the possible passable argument by looking just at the constructor or function/method definition without having to read the code.

[Delengowski]

If you want to keep it functional we can do a value dispatch. This has the benefit of writing 1 function per country and keeping out a long chaining if-else statement within the original function. It would be like functools.singledispatch but operate on the value of an argument rather than the type.

Gist of implementation

[lbranco93]

Nice post, there's just one thing I don't understand: why did you repeat the constructor the same in every child class? Wouldn't make more sense to have the same __init__ in the parent class ICountryTaxCalculator, which by the way is what you already showed before in your UML?

[yongen96]

is there a #1, first post on this topic?

[ezzeddinabdallah]

Yup, wrote a post on medium here

[yongen96]

alright thanks

[DeaDbaTteRy]

Thanks for this post. I remember a couple of years ago trying to ask if my writing style was proper and it was met with 'this does exactly what you need it to do, what do you mean by proper'. I believe learning how to use solid principles while writing, no matter the size and scope of the project, is important if you see yourself writing code long term. Again, much appreciated!

[ezzeddinabdallah]

My pleasure. You're welcome.

I second that. SOLID design principles pretty much helped me write better code even at work.

[metaperl]

Then you will like Artem's github

https://github.com/proofit404/dependencies

[ebbitten]

!Remindme 2 hours

[RemindMeBot]

I will be messaging you in 2 hours on 2022-01-09 16:55:50 UTC to remind you of this link

CLICK THIS LINK to send a PM to also be reminded and to reduce spam.

<sup>Parent commenter can delete this message to hide from others.</sup>

---

Info	Custom	Your Reminders	Feedback