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?

"They understand that they violate the SRP". I can't hardly believe they understand this. SRP is means to an end. This end is maintainability. It's often hard to see the benefit of splitting classes when looking at such a small example in isolation. — Steven, Oct 12 '16 at 14:41
Yes. It is clear that problems come when the system grows. But they believe that it is overkill to create a new class for every type of report. Like it is easier to add new method every time. I just have no more ideas how to make it clear that the system should be scalable from the very beginning ) — Adilet Maratov, Oct 13 '16 at 07:08
This question probably belongs to programmers rather than SO. — plalx, Oct 14 '16 at 19:36
To me, they are 3 different responsibilities and should be separated. I am currently learning that, there is another perspective from which to look at this. That being "Change". If you expect that Change to occur, then go ahead and split the responsibilities. If you are absolutely certain, a particular change is not going to happen and one desires minimizing complexity, let it be. — ShaQ.Blogs, Nov 28 '16 at 18:24

score 1 · Accepted Answer · answered Jan 05 '17 at 21:57

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()))

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

1 Answers1