Why does too-many-statements consider statements in nested functions?

[schiele]

Question

If I write a lengthy function, the too-many-statements checker rightfully complains about that. The obvious solution is to factor out code into separate functions. If those functions are not useful outside this function, I nest them into the function calling them. If I do that, the checker still considers those statements for complaining about the outer function. Is this a bug in the checker implementation or is there a good reason the checker discourages form using nested functions?

Documentation for future user

Documentation should elaborate on what it includes in the calculation on the documentation page https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/too-many-statements.html.
For not obvious cases, like nested code, it should also provide rationale about the decision.

Additional context

No response

[jacobtylerwalls]

This is a good question -- it's either a bug or feature. I think it makes sense to keep the behavior as is, but I'm eager to hear other views, too.

Nested functions/methods have a place, but they still contribute to the complexity of the design. For instance they're harder to patch when testing, harder to subclass, harder to wrap with functools.partial, etc. As a developer you will need to weigh this against the benefit you get from hiding them in a closure. (We also wouldn't want to encourage people to just pass a lint check by needlessly creating nested functions.)

[Pierre-Sassoulas]

I'm torn. The instinctive way I would "fix" a function that is too complex but with nothing that is going to be reused elsewhere is to create a module with only the function and it's helper function. Then I might make the helper function private.

def print_cats():
    print("Cats are amazing creatures.")
    print("They have soft fur and sharp claws.")
    print("Cats can be very playful or very lazy.")
    print("Some cats love to cuddle, while others prefer to be independent.")
    print("No matter their personality, cats are wonderful companions!")

=>

__all__ = ["print_cats"]

def _describe_cats():
    print("Cats are amazing creatures.")
    print("They have soft fur and sharp claws.")

def _cat_behavior():
    print("Cats can be very playful or very lazy.")
    print("Some cats love to cuddle, while others prefer to be independent.")

def _cat_conclusion():
    print("No matter their personality, cats are wonderful companions!")

def print_cats():
    _describe_cats()
    _cat_behavior()
    _cat_conclusion()

Which isn't necessarily better or worse than this:

def print_cats():
    def describe_cats():
        print("Cats are amazing creatures.")
        print("They have soft fur and sharp claws.")

    def cat_behavior():
        print("Cats can be very playful or very lazy.")
        print("Some cats love to cuddle, while others prefer to be independent.")

    def cat_conclusion():
        print("No matter their personality, cats are wonderful companions!")

    describe_cats()
    cat_behavior()
    cat_conclusion()

Or this:

class CatPrinter:
    @staticmethod
    def describe_cats(self):
        print("Cats are amazing creatures.")
        print("They have soft fur and sharp claws.")

    @staticmethod
    def cat_behavior(self):
        print("Cats can be very playful or very lazy.")
        print("Some cats love to cuddle, while others prefer to be independent.")

    @staticmethod
    def cat_conclusion(self):
        print("No matter their personality, cats are wonderful companions!")

    @staticmethod
    def print_cats(self):
        self.describe_cats()
        self.cat_behavior()
        self.cat_conclusion()

[jacobtylerwalls]

What do you think of the point about overriding and mock.patch'ing?

[Pierre-Sassoulas]

On second thought, clearly if you have 50+ lines of code, you might want to unit test some of it and having inner function is the one solution that prevent to do that.