65

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?

6
  • 1
    "having too many functions is bad practice."? Why? Please update your question to explain why this seems bad to you.
    – S.Lott
    Feb 14, 2011 at 11:07
  • I'd recommend reading into the notion of 'class cohesiveness' - Bob Martin discusses this in 'Clean Code'. Sep 19, 2018 at 9:47
  • Adding to other answers, split and name, but let us all keep in mind that, as a class grows, its member variables become more and more like global variables. One way to ameliorate the problem (apart from better solutions such as refactoring ;-) ) is to separate those little private methods into stand-alone functions if that is possible (in that they do not need access to lot of state). Some languages allow out-of-class functions, others have static classes. In this way this way you can understand that function in isolation.
    – Pablo H
    Oct 5, 2018 at 12:56
  • If anyone can tell me why "this answer is not useful", I'd be grateful. One can always learn. :-)
    – Pablo H
    Oct 11, 2018 at 13:19
  • @PabloH The answer you provided gives a good observation and insight to the OP, however it does not actually answer the question that was asked. I moved it into the comments field.
    – maple_shaft
    Oct 11, 2018 at 15:11

10 Answers 10

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.

7
  • 3
    So you believe it's good 'encapsulation' to make logic available to an entire class which is only called in one place? Feb 14, 2011 at 8:22
  • 10
    @Steven Jeuris: As long as those functions are made private, I see no problem with it; in fact, I find that, as stated in this answer, easier to read. It is much easier to understand a high-level public function which only calls a series of low-level private functions (which have been appropriatly named, of course). Sure, all that code could have been put in the high-level function itself, but then it would take longer to get an understanding of the code as you have to look through much more of it at one and the same place.
    – gablin
    Feb 14, 2011 at 8:47
  • 2
    @gablin: private functions are still accessible from within the entire class. In this (apparently controversial) article I discuss how 'global readability' is lost when having too much functions. Actually a common principle besides that functions should do only one thing, is you shouldn't have too many. You only gain 'local readability' by splitting just for shortness, which can also be achieved with simple comments and 'code paragraphs'. Ok, code should be self-documenting, but comments aren't obsolete! Feb 14, 2011 at 12:59
  • 1
    @Steven - What's easier to understand? myString.replaceAll(pattern, originalData); or me pasting the entire replaceAll method in my code, including the backing char array that represents the underlying string object each and every time I need to use the functionality of a String?
    – jmort253
    Feb 15, 2011 at 6:35
  • 7
    @Steven Jeuris: You do have a point. If a class has too many functions (public or private), then maybe the entire class is trying to do too much. In such cases, it may be better to split it up into several smaller classes, just as you would split up too large a function into several smaller functions.
    – gablin
    Feb 15, 2011 at 11:49
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

7
  • 1
    It's nice to read not everyone follows the 'one thing' rule blindly! ;p Nice answer! Feb 14, 2011 at 8:27
  • 16
    I often break up a large method into smaller methods even though they will only serve the bigger method. The reason for dividing it up is because it becomes easier to handle and understand, instead of just having one huge blob of code (which may or may not be self- and well commented).
    – gablin
    Feb 14, 2011 at 8:41
  • 1
    That's ok too in some instances where you really can't avoid a large method. For instance, if you have an Initialize method you might have it call InitializeSystems, InitializePlugins etc etc. One should always keep checking oneself that refactoring isn't needed though
    – Homde
    Feb 14, 2011 at 11:17
  • 1
    The key thing about every method (or function) is that it must have a clear conceptual interface, it's “contract”. If you cannot pin that down, it's going to be trouble and you've got the factoring wrong. (Explicitly documenting the contract can be a good thing – or using a tool to enforce it, Eiffel-style – but it's not as important as having it there in the first place.) Feb 14, 2011 at 11:51
  • 3
    @gablin: another benefit to consider: when I break things up into smaller functions, don't think "they only serve one other function", think "they only serve one other function for now". There have been several occasions where re-factoring large methods have saved me heaps of time when writing other methods in the future because the smaller functions were more generic and reusuable.
    – GSto
    Feb 14, 2011 at 16:03
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.

10

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.

1
  • 1
    I think this is a great answer and is a direction that the code could surely go. Best practices say to use the most restrictive access modifier that gives you the functionality you need. If the methods aren't used outside the class, private makes the most sense. If it makes more sense to make the methods public or move them to another class so they can be used elsewhere, then by all means, that can be done as well. +1
    – jmort253
    Feb 15, 2011 at 6:41
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.

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.

1
  • +1 for "You gain nothing if those private methods are just pieces of an arbitrarily divided long story, each being meaningless outside the whole context."
    – Mahdi
    Feb 14, 2014 at 8:07
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.

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.

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.

1
  • 1
    How can it break the open closed principle? The code is both open for extension and closed for modification regardless of whether the public methods have been split into smaller private ones.
    – byxor
    Jan 7, 2017 at 14:50

Not the answer you're looking for? Browse other questions tagged or ask your own question.