(Not) creating private methods
The other day, my friend asked me about my opinion on whether to extract private methods from a bigger one or not. We both read John Ousterhout’s A Philosophy of Software Design, which suggests tending to keep code as one big flow when the logical substeps of it are not reusable. It might sound provoking, but there indeed might be a cognitive load increase if you have to jump back and forth to method definitions in order to see what’s happening in there. But there is a risk of such a big method becoming a maintenance hell quite easily. So, I decided to collect my thougths here (in no particular order) to understand what my opinion about this actually is.
For starters, there’s a mandatory detour to the question whether a method which does multiple things is not actually a violation of the single responsibility principle. Well - it might be. But having single responsibility does not necessarily mean it only does one thing (in that case, every repository class would be violating it, and we’d need to have separate classes CreateEntity, DeleteEntity etc). Rather, it should have one reason to change. But although this is a good thought, distilling the responsibilities well is still a very hard design task. It’s not that hard to realize that some “detailed” operations should probably not be all in one god object - but even when we create specific classes tasked with their very limited and well-bounded responsibilities, there still need to be higher-layer classes connecting the pieces together, and judging whether their responsibility is not too wide (whether they indeed have just one reason to change, if their job is stitching multiple other operations together) is rather tricky in my opinion - at least with my current level of object design skills, I consider this a challenge.
Let’s say for now we’re at peace with how our class fulfills the SRP. Even then, it has one public method with pretty large body and no other methods. Is this a good thing? I think this very much depends on the experience of the programmer(s) involved. John Ousterhout’s argument about how the flow can nicely read from top to bottom in one go is only valid as long as the code has very good quality. And after coming as far with writing this article, I feel like I found what I think is most important for me when deciding how to split the method. Let me elaborate:
As our code is constantly evolving, there can easily come a situation when a piece of code which we highly doubted to be ever needed elsewhere suddenly is indeed. At this point, we open up our big method and decide to extract the piece. For a very senior programmer, this might be a piece of cake (pun semi-intended) - they just take the few incriminating lines, put them elsewhere and call it a day. However, this is the ideal scenario - depends on 1. the preexisting being very well structured, and 2. the extracting programmer to be just as good as the author of the code (quite likely this won’t be the same person). And if the situation is not as ideal, extracting the code might prove quite daunting if the code is not well structured, and / or the extractor can take out less / more than necessary, which might only become apparent after some time, when the cost of change will be much higher. Furthermore, if the extraction seems too daunting, the programmer might give up entirely and write similarly behaving code elsewhere, which might be a terrible decision only time will reveal. Especially if those two pieces of code seem too different, even a brave refactoring-pursuing programmer coming at a later stage, willing to improve the situation, might not understand these two pieces of code should actually be the same one - and maybe they aren’t anymore, if they (accidentally) diverged over time.
If you get an itchy feeling when reading this, chances are you’ve recently encountered a very similar situation yourself. I feel your pain :) we’ve all been there.
So from this perspective, dividing the big method into smaller chunks early on might be a safer play - also because while doing so, the scope and dependencies of each of the methods will become more “in your face” and help you see it more clearly, thus allowing more refactoring and clearer responsibilities early-on. Also, these smaller methods might be testable separately (this might not be applicable for you, depending on your testing approach), which can further helps you increase the trust in that specific, well-isolated piece of code - and when writing the test, there’s a whole another layer of “in your face-ness” regarding the dependencies and scope, giving you yet another opportunity to reevaluate. All of this before another programmer comes to the table and needs to extract something - if you make it so easy for them, chances are they won’t even have a chance of messing up.
One interesting approach I’d like to mentioned is the composed method pattern by Kent Beck - a method which calls several other methods, each at the same level of detail as one another. This wasn’t intentional when I begun, but here I’m actually coming back to the topic of a “higher-layer classes connecting the pieces together” as mentioned above, even though there I meant it as cross-class while the composed method often works with private methods of the same class. If done well (and the private methods are named very explicitly), I believe such a method can provide a similar easiness of reading as what John Ousterhout suggested.
I’ll conclude with a related story - a couple of weeks ago, I was adding some functionality into a preexisting class I did not write. I added it happily and pushed the code, only to find out our linter does not like that it exceeds a specific number of lines. I didn’t see much opportunity to shorten it (extracting just my newly added code to a private method wouldn’t have helped enough), so I asked for reevaluation of this rule and it was almost successful, but then I went to reevaluate if I really couldn’t shorten it. To do that, I needed to actually refactor the preexisting code, which at the beginning I really wanted to avoid, but with some effort I was able to extract some private methods successfully. Why am I mentioning this? The result was indeed better than if the linter didn’t push me to reconsider. I know LoC-based linter rules are polarizing, but it made a difference for the better in the end.