Should I add new methods to a class instead of using Single Responsibility Principle

Question

We had a seminar where I presented Single Responsibility Principle to my team so that we use it in our projects. I used the following popular example:

class Employee:
    save()
    calculate_salary()
    generate_report()

And I asked the team to tell whether everything is okay with this class. Everybody told me that it's okay. But I see three violations of the SRP principle here. Am I right if I say that all methods should be extracted from the class? My reasoning:

save() method is a reason for change if change our database.

calculate_salary() method is a reason for change because the salary policy may change.

generate_report() method is a reason for change if we want to change the presentation of the report (i.e. csv instead of html).

Let's take the last method. I came up with the following HtmlReportGenerator class.

class HTMLReportGenerator:
    def __init__(self, reportable):
        self.reportable = reportable

    def generate_csv_report()

class CSVReportGenerator:
    def __init__(self, reportable):
        self.reportable = reportable

    def generate_html_report()

Now even if business logic of this generator changes, it does not touch the Employee class and this was my main point. Moreover, now we can reuse those classes to objects other than Employee class objects.

But the team came up with a different class:

class Employee:
    save()
    calculate_salary()
    generate_html_report()
    generate_csv_report()

They understand that they violate the SRP, but it is okay for them.

And this is where I had no other ideas to fight for ))

Any ideas on the situation?

Answer 1

I agree with you, by adding additional functions they violated both the SRP and the open/close principle, and every time there will be a new report type they will violate it again.

I would keep the generate_report() function but add a parameter from interface Type "ReportType" which has a function generate().

This means that for example you can call (pardon my Java):

employee.generate_report(new CSVReport())

employee.generate_report(new HTMLReport())

And tomorrow if you want to add a XML report you just implement XMLReport from the Report interface and call :

employee.generate_report(new XMLReport())

This gives you a lot of flexibility, no need to change employee for new report types, and much easier to test (for example if generate_report had complicated logic you could just create a TestReport class that implements Report interface and just prints to the output stream for debugging and call generate_report(new TestReport()))