Are private methods with a single reference bad style?

Question

Generally I use private methods to encapsulate functionality that is reused in multiple places in the class. But sometimes I have a large public method that could be broken up into smaller steps, each in its own private method. This would make the public method shorter, but I'm worried that forcing anyone who reads the method to jump around to different private methods will damage readability.

Is there a consensus on this? Is it better to have long public methods, or break them up into smaller pieces even if each piece is not reusable?

Answer 1

No, this is not a bad style. In fact it is a very good style.

Private functions need not exist simply because of reusability. That is certainly one good reason to create them, but there is another: decomposition.

Consider a function that does too much. It is a hundred lines long, and impossible to reason about.

If you split this function into smaller pieces, it still "does" as much work as before, but in smaller pieces. It calls other functions which should have descriptive names. The main function reads almost like a book: do A, then do B, then do C, etc. The functions it calls may only be called in one place, but now they are smaller. Any particular function is necessarily sandboxed from the other functions: they have different scopes.

When you decompose a large problem into smaller problems, even if those smaller problems (functions) are only used/solved once, you gain several benefits:

• Readability. Nobody can read a monolithic function and understand what it does completely. You can either keep lying to yourself, or split it up into bite-sized chunks that make sense.

• Locality of reference. It now becomes impossible to declare and use a variable, then have it stick around, and used again 100 lines later. These functions have different scopes.

• Testing. While it is only necessary to unit test the public members of a class, it may be desirable to test certain private members as well. If there is a critical section of a long function that might benefit from testing, it is impossible to test it independently without extracting it to a separate function.

• Modularity. Now that you have private functions, you may find one or more of them that could be extracted into a separate class, whether it is used only here or is reusable. To the previous point, this separate class is likely to be easier to test as well, since it will need a public interface.

The idea of splitting big code into smaller pieces that are easier to understand and test is a key point of Uncle Bob's book Clean Code. At the time of writing this answer the book is nine years old, but is just as relevant today as it was back then.

Answer 2

It's probably a great idea!

I do take issue with splitting up long linear sequences of action into separate functions purely to reduce the average function length in your codebase:

function step1(){
  // ...
  step2(zarb, foo, biz);
}

function step2(zarb, foo, biz){
  // ...
  step3(zarb, foo, biz, gleep);
}

function step3(zarb, foo, biz, gleep){
  // ...
}

Now you've actually added lines of source and reduced total readability considerably. Especially if you're now passing lots of parameters between each function to keep track of state. Yikes!

However, if you've managed to extract one or more lines into a pure function that serves a single, clear purpose (even if called only once), then you've improved readability:

function foo(){
  f = getFrambulation();
  g = deglorbFramb(f);
  r = reglorbulate(g);
}

This likely won't be easy in real-world situations, but pieces of pure functionality can often be teased out if you think about it long enough.

You'll know you're on the right track when you have functions with nice verb names and when your parent function calls them and the whole thing practically reads like a paragraph of prose.

Then when you return weeks later to add more functionality and you find that you can actually re-use one of those functions, then oh, rapturous joy! What wondrous radiant delight!

Answer 3

The answer is it highly depends on the situation. @Snowman covers the positives of breaking up large public function, but its important to remember there can be negative effects too, as you are rightly concerned about.

• Lots of private functions with side-effects can make code difficult to read and quite fragile. This is especially true when these private functions depend upon the side effects of each other. Avoid having tightly-coupled functions.

• Abstractions are leaky. While its nice to pretend how an operation is performed or how data is stored doesn't matter, there are cases where it does, and it's important to identify them.

• Semantics and context matter. If you fail to clearly capture what a function does in its name, you again can reduce readability and increase fragility of the public function. This is especially true when you start creating private functions with large numbers of input and output parameters. And while from the public function, you see what private functions it calls, from the private function, you don't see what public functions call it. This can lead to a "bug-fix" in a private function that breaks the public function.

• Heavily decomposed code is always clearer to the author than to others. This doesn't mean that it can't still be clear to others, but it's easy to say that it makes perfect sense that bar() must be called before foo() at the time of writing.

• Dangerous re-use. Your public function likely restricted what input was possible to each private function. If these input assumptions are not properly captured (documenting ALL assumptions is hard), someone may improperly re-use one of your private functions, introducing bugs into the codebase.

Decompose functions based on their internal cohesion and coupling, not absolute length.

Answer 4

IMHO, the value of pulling out blocks of code purely as a means of breaking up complexity, would often be related to the difference in complexity between:

1. A complete and accurate human-language description of what the code does, including how it handles corner cases, and

2. The code itself.

If the code is a lot more complicated than the description, replacing the in-line code with a function call may make the surrounding code easier to understand. On the other hand, some concepts can be expressed more readably in a computer language than in human language. I would regard w=x+y+z;, for example, as more readable than w=addThreeNumbersAssumingSumOfFirstTwoDoesntOverflow(x,y,z);.

As a large function gets split up, there will be less and less difference there is between the complexity of sub-functions versus their descriptions, and the advantage of further subdivisions will decrease. If things are split up to the point that descriptions would be more complicated than code, further splits will make the code worse.

Answer 5

It is a balancing challenge.

Finer is better

The private method effectively provides a name for the contained code, and sometimes a meaningful signature (unless half of its parameters are completely ad-hoc tramp data with unclear, undocumented interdependences).

Giving names to code constructs is generally good, as long as the names suggest a meaningful contract for the caller, and the contract of the private method accurately corresponds to what the name suggests.

By forcing oneself to think of meaningful contracts for smaller portions of the code, the initial developer can spot some bugs and avoid them painlessly even before any dev testing. This only works if the developer is striving for concise naming (simple sounding but accurate) and is willing to adapt the boundaries of the private method so that concise naming is even possible.

Subsequent maintenance is helped, too, because additional naming helps make the code self-documenting.

• Contracts over small pieces of code
• Higher level methods sometimes turn into a short sequence of calls, each of which does something significant and distinctly named - accompanied with the thin layer of general, outermost error handling. Such a method can become a valued training resource for anyone who has to quickly become an expert on the overall structure of the module.

Until it gets too fine

Is it possible to overdo giving names to small chunks of code, and end up with too many, too small private methods? Sure. One or more of the following symptoms indicate that the methods are getting too small to be useful:

• Too many overloads that don't really represent alternative signatures for the same essential logic, but rather a single fixed call stack.
• Synonyms used just to refer to the same concept repeatedly ("entertaining naming")
• Lots of perfunctory naming decorations such as XxxInternal or DoXxx, especially if there's no unified scheme in introducing those.
• Clumsy names almost longer than the implementation itself, like LogDiskSpaceConsumptionUnlessNoUpdateNeeded

Answer 6

Contrary to what the others have said, I would argue that a long public method is a design smell that is not rectified by decomposing into private methods.

But sometimes I have a large public method that could be broken up into smaller steps

If this is the case then I would argue each step should be its own first class citizen with a single responsibility each. In an object-oriented paradigm, I would suggest creating an interface and an implementation for each step in such a way that each has a single, easily-identifiable responsibility and can be named in a way that the responsiblity is clear. This allows you to unit-test the (formerly) large public method, as well as each individual step independently of each other. They should all be documented as well.

Why not decompose into private methods? Here are a few reasons:

• Tight coupling and testability. By reducing your public method's size, you have improved its readability, but all the code is still tightly coupled together. You can unit test the individual private methods (using advanced features of a testing framework), but you cannot easily test the public method independently of the private methods. This goes against the principles of unit testing.
• Class size and complexity. You've reduced the complexity of a method, but you've increased the complexity of the class. The public method is easier to read, but the class is now more difficult to read because it has more functions that define its behavior. My preference is for small single-responsibility classes, so a long method is a sign that the class is doing too much.
• Cannot be reused easily. It's often the case that as a body of code matures reusability is useful. If your steps are in private methods, they cannot be reused anywhere else without first extracting them somehow. Additionally it may encourage copy-paste when a step is needed elsewhere.
• Splitting in this way is likely to be arbitrary. I would argue that splitting a long public method doesn't take as much thought or design consideration as if you were to break up responsibilties into classes. Each class must be justified with a proper name, documentation, and tests, whereas a private method doesn't get as much consideration.
• Hides the problem. So you've decided to split your public method into small private methods. Now there is no problem! You can keep adding more and more steps by adding more and more private methods! On the contrary, I think this is a major problem. It establishes a pattern for adding complexity to a class that will be followed by subsequent bug fixes and feature implementations. Soon your private methods will grow and they will have to be split.

but I'm worried that forcing anyone who reads the method to jump around to different private methods will damage readability

This is an argument I've had with one of my colleagues recently. He argues that having the entire behavior of a module in the same file/method improves readability. I agree that the code is easier to follow when all together, but the code is less easy to reason about as complexity grows. As a system grows, it become intractable to reason about the entire module as a whole. When you decompose complex logic into several classes each with a single responsiblity, then it becomes much easier to reason about each part.