66

I understand the importance of well documented code. But I also understand the importance of self-documenting code. The easier it is to visually read a particular function, the faster we can move on during software maintenance.

With that said, I like to separate big functions into other smaller ones. But I do so to a point where a class can have upwards of five of them just to serve one public method. Now multiply five private methods by five public ones, and you get around twenty-five hidden methods that are probably going to be called only once by those public ones.

Sure, it's now easier to read those public methods, but I can't help but think that having too many functions is bad practice.

[Edit]

People have been asking me why I think having too many functions is bad practice.

The simple answer: it's a gut feeling.

My belief is not, for one bit, backed by any hours of software engineering experience. It's just an uncertainty that gave me a "writer's block", but for a programmer.

In the past, I have only been programming personal projects. It's just recently that I moved on to team-based projects. Now, I want to ensure that others can read and understand my code.

I wasn't sure what will improve legibility. On one hand, I was thinking of separating one big function into other smaller ones with intelligible names. But there was another side of me saying that it's just redundant.

So, I'm asking this to enlighten myself in order to pick the correct path.

[Edit]

Below, I included two versions of how I could solve my problem. The first one solves it by not separating big chunks of code. The second one does separate things.

First version:

public static int Main()
{
    // Displays the menu.
    Console.WriteLine("Pick your option");
    Console.Writeline("[1] Input and display a polynomial");
    Console.WriteLine("[2] Add two polynomials");
    Console.WriteLine("[3] Subtract two polynomials");
    Console.WriteLine("[4] Differentiate two polynomials");
    Console.WriteLine("[0] Quit");
}

Second version:

public static int Main()
{
    DisplayMenu();
}

private static void DisplayMenu()
{
    Console.WriteLine("Pick your option");
    Console.Writeline("[1] Input and display a polynomial");
    Console.WriteLine("[2] Add two polynomials");
    Console.WriteLine("[3] Subtract two polynomials");
    Console.WriteLine("[4] Differentiate two polynomials");
    Console.WriteLine("[0] Quit");
}

In the above examples, the latter calls a function that will only be used once throughout the program's entire runtime.

Note: the code above is generalized, but it's of the same nature as my problem.

Now, here's my question: which one? Do I pick the first one, or the second one?

Sal Rahman
  • 1,544

10 Answers10

36

Now multiply five private methods by five public ones, and you get around twenty-five hidden methods that are probably going to be called only once by those public ones.

This is what is known as Encapsulation, which creates a Control Abstraction at the higher level. This is a good thing.

This means that anyone reading your code, when they get to the startTheEngine() method in your code, can ignore all of the lower level details such as openIgnitionModule(), turnDistributorMotor(), sendSparksToSparkPlugs(), injectFuelIntoCylinders(), activateStarterSolenoid(), and all of the other complex, small, functions that must be run in order to facilitate the much larger, more abstract function of startTheEngine().

Unless the problem you are dealing with in your code deals directly with one of those components, code maintainers can move on, ignoring the sandboxed, encapsulated functionality.

This also has the added advantage of making your code easier to test.. For instance, I can write a test case for turnAlternatorMotor(int revolutionRate) and test it's functionality completely independent of the other systems. If there is a problem with that function and the output isn't what I expect, then I know what the problem is.

Code that isn't broken down into components is much harder to test. Suddenly, your maintainers are only looking at the would be abstraction instead of being able to dig down into measurable components.

My advice is to keep doing what you're doing as your code will scale, be easy to maintain, and can be used and updated for years to come.

jmort253
  • 9,347
22

Yes and no. The fundamental tenet is: a method should do one thing and one thing only

It's correct to "break up" your method into smaller methods. Then again, those methods should optimally be general enough not to only serve that "big" method but be reusable in other places. In some cases a method will follow a simple tree structure though, like an Initialize Method that calls InitializePlugins, InitializeInterface etc.

If you have a really big method/class it's usually a sign that it's doing to much and you need to do some refactoring to break up the "blob". Perphaps hide some complexity in another class under an abstraction, and do use dependency injection

Homde
  • 11,114
18

I think in general it's better to err on the side of too many functions rather than too few. The two exceptions to that rule I have seen in practice are for the DRY and YAGNI principles. If you have a lot of nearly identical functions, you should combine them and use parameters to avoid repeating yourself. You can also have too many functions if you created them "just in case" and they aren't being used. I see absolutely nothing wrong with having a function that is only used once if it adds readability and maintainability.

Karl Bielefeldt
  • 148,830
11

jmort253's great answer inspired me to follow up with a "yes, but" answer...

Having lots of small private methods isn't a bad thing, but pay attention to that niggling feeling that's making you post a question here :). If you get to a point where you're writing tests which are focussed just on private methods (either by calling them directly, or by setting up a scenario such that one is called in a certain way just so you can test the result) then you should take a step back.

What you may have is a new class with its own more focussed public interface trying to escape. At that point you might want to think about extracting a class out to encapsulate that part of your existing classes functionality that's currently living in private method. Now your original class can use your new class as a dependency, and you can writing more focussed tests against that class directly.

8

Almost every OO project I have ever joined suffered badly from classes that were too large, and methods that were too long.

When I feel the need for a comment explaining the next section of code, then I extract that section into a separate method. In the long run, this makes the code shorter. Those little methods get reused more than you would initially expect. You start seeing opportunities to collect a bunch of them into a separate class. Soon you are thinking about your problem at a higher level, and the whole effort goes much faster. New features are easier to write, because you can build on those little classes you created from those little methods.

kevin cline
  • 33,798
7

A class with 5 public methods and 25 private methods doesn't seem that large to me. Just make sure your classes has well-defined responsibilities and don't worry too much about the number of methods.

That said, those private methods should focus on one specific aspect of the whole task, but not in a "doSomethingPart1", "doSomethingPart2" manner. You gain nothing if those private methods are just pieces of an arbitrarily divided long story, each being meaningless outside the whole context.

user281377
  • 28,434
5

Excerpt from R. Martin's Clean Code (p. 176):

Even concepts as fundamental as elimination of duplication, code expressiveness, and the SRP can be taken too far. In an effort to make our classes and methods small, we might create too many tiny classes and methods. So this rule [minimize the number of classes and methods] suggests that we also keep our function and class counts low. High class and method counts are sometimes the result of pointless dogmatism.

3

Some options:

  1. That's fine. Just name the private methods as well as you name your public methods. And name your public methods well.

  2. Abstract out some of these helper methods into helper classes or helper modules. And make sure these helper classes / modules are named well and are solid abstractions in and of themselves.

yfeldblum
  • 1,542
1

I don't think there is ever a problem of "too many private methods" if it feels that way it is probably a symptom of poor code architecture.

Here is how I think about it... If the architecture is well designed, it's generally obvious where the code that does X should be. It's acceptable if there are a couple of exceptions to this - but these need to be truly exceptional otherwise refactor the architecture to handle these as standard.

If the issue is that on opening your class, it is full of private methods that are breaking bigger chunks into smaller chunks of code, then perhaps this is a symptom of missing architecture. Instead of classes and architecture, this area of code has been implemented in a procedural way inside one class.

Finding a balance here depends on the project and its requirements. The counter argument to this is the saying that a junior programmer can knock up the solution in 50 lines of code that takes an architect 50 classes.

Michael Shaw
  • 10,114
0

Is there such a thing as having too many private functions/methods?

Yes.

In Python the concept of "private" (as used by C++, Java and C#) doesn't really exist.

There's a "sort-of-private" naming convention, but that's it.

Private can lead to difficult-to-test code. It can also break the "Open-Closed Principle" by making code simply closed.

Consequently, for folks who've used Python, private functions have no value. Just make everything public and be done with it.

S.Lott
  • 45,522
  • 6
  • 93
  • 155