Kotlin - Nested functions vs private function with 1 callsite

Viewed 4397

Whenever I have a function which has gotten large enough to warrant decomposing into smaller functions, I always go for creating the smaller functions as nested functions of the larger functions scope like so:

class Foo {
    fun bar() : Int {
        fun a() : Int {
            // Do a load of stuff
            return 1
        }
        fun b() : Int {
            // Do a load of stuff
            return 1
        }

        return a() + b()
    }
}

I do this as it provides encapsulation of these functions which as of yet only have a single callsite; that of the enclosing scope.

However I am frequently asked at work to refactor these functions out to private functions of the enclosing class like so:

class Foo {
    fun bar() : Int {
        return a() + b()
    }

    private fun a() : Int {
        // Do a load of stuff
        return 1
    }

    private fun b() : Int {
        // Do a load of stuff
        return 1
    }
}

My argument against this is that these functions only have 1 callsite, and by hoisting them to class level private functions I am muddying the class with methods that are only called in one place.

An additional minor argument can also be made that if I make them private functions of the class, someone can come in later and start inserting methods between those private functions and the function that calls them, such that there could be 100's of lines of code between the callsite and the functions themselves, causing mental gymnastics to be required to understand the calling function (as you now need to scroll the calling function off the screen to see the private functions).

I always comply and move them to private functions of the class after my argument doesn't pursuade the reviewer(s).

Is my argument valid or are there legitimate reasons (performance, code readability) that make my argument invalid?

1 Answers

I find very confusing to transform nested functions into private functions within the class that contains the function, since the only purpose is just to improve the readability of the main function. The private clause only protects against inadvertent use outside the class, but it hinders the readability of the class's source code.

In terms of efficiency, there are no problems, quite the opposite, but it's irrelevevant. In compiler time the search for valid functions within the class is more streamlined, since the nested functions are not even part of the data dictionary.

In terms of efficiency, there is a small overhead when allowing the nested function to see part of the context of the parent function, which is in an earlier section of the code. Since use is only as a means of achieving better readability, I see no problem with that.

As a private function, the compiler can access the function anywhere in the class, which I think is an encapsulation flaw, since that is not the purpose of a local and nested function.

Related