Does logging inside a class violate the SRP

design-patternsloggingobject-oriented-designsingle-responsibility

I wrote a class that takes a Logger class as one of its arguments:

class QueryHandler:

    def __init__(self, query: Query, logger: Logger) -> None:
        self.query = query
        self.logger = logger

    def run(self) -> None:

        try:
            r = requests.get(self.query.url)
        except requests.exceptions.RequestException as err:
            raise QueryException(f'Failed to complete {self.query.id}') from err  # log outside in caller
        else:
            sc = r.status_code
            if sc != 200:
                self.logger.warning(f'{self.query.id} returned with code: {sc}')
            else:
                self.query.response = bytearray(r.content)

There is other functionality, but a colleague, specifically focused on this method, cites that, because the class calls the self.logger.warning, it violates the single responsibility principle (SRP). My position was that the class is responsible for handling a particular query object, and when run is called, by calling the self.logger.warning it is not a violate of the SRP because the class it not concerned with how the logging takes place, that is delegated to the Logger class. I further argued that the SRP doesn't apply to functions, and thereby methods (i.e., bound functions).

If the run method implemented the logging logic (opening a file, formatting the message, etc.), then I would agree with my colleague.

Is my understanding of the SRP incorrect?

Best Answer

Your colleague is taking the word "Single" too literally and dogmatically.

SRP just means a class should have a single conceptual purpose. What constitutes a single conceptual purpose will differ between different types of software, and how the module fits in a larger program. There isn't some sort of mechanical definition you can test against in a vacuum, it's a judgement call for humans to decide.

When you see a class called File, you should be able to expect it to only deal with files. If you see a class called Random you should expect it to only be for randomization purposes. You wouldn't expect it also to write to Files, or open network connections, or parse Urls or something.

Any class that does anything useful is going to do multiple things if you treat "responsibility" too simply. Any if statement will result in two possible code paths, thus anything with it will be doing at least two things, but an entire program without any conditionals isn't going to be terribly useful for many applications.

Including logging in a class is normally not sensibly considered a violation of SRP. In an example of File, Random, or ExpensiveMachineController class, I would not expect the logging behavior to have any influence over how those classes behave, with regards to how I'm hooking them up to other classes. Logging would provide me a diagnostic function, and I should be able to ignore that completely and still use those classes correct.

Sure, there could be bugs and strange edge cases could cause weirdness, but avoiding that is a big part of the craft of software development, and the design of the logging should prevent that.

Related Topic