Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
2  
This is an important question. Vote it up if you agree. –  gablin Feb 14 '11 at 8:38
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 '11 at 11:07

8 Answers 8

up vote 12 down vote accepted

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.

share|improve this answer
1  
So you believe it's good 'encapsulation' to make logic available to an entire class which is only called in one place? –  Steven Jeuris Feb 14 '11 at 8:22
4  
@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 '11 at 8:47
1  
@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! –  Steven Jeuris Feb 14 '11 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 '11 at 6:35
3  
@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 '11 at 11:49

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.

share|improve this answer

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

share|improve this answer
    
It's nice to read not everyone follows the 'one thing' rule blindly! ;p Nice answer! –  Steven Jeuris Feb 14 '11 at 8:27
11  
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 '11 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 –  konrad Feb 14 '11 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.) –  Donal Fellows Feb 14 '11 at 11:51
2  
@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 '11 at 16:03

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.

share|improve this answer

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.

share|improve this answer
    
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 '11 at 6:41

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.

share|improve this answer
    
+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 at 8:07

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.

share|improve this answer

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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