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?