Small functions vs. keeping dependent functionality in same function

Question

I have a class that sets up an array of nodes and connects them to each other in a graph-like structure. Is it best to:

1. Keep the functionality to initialize and connect the nodes in one function
2. Have the initialization and connection functionality in two different functions (and have a dependent order on which the functions must be called -- though keep in mind these functions are private.)

Method 1: (Bad in that one function is doing two things, BUT it keeps dependent functionality grouped together -- the nodes should never be connected without being initialized first.)

init() {
    setupNodes()
}

private func setupNodes() {
    // 1. Create array of nodes
    // 2. Go through array, connecting each node to its neighbors 
    //    according to some predefined constants
}

Method 2: (Better in the sense that it is self-documenting, BUT connectNodes() should never be called before setupNodes(), so anyone working with the class internals needs to know about this order.)

init() {
    setupNodes()
}

private func setupNodes() {
    createNodes()
    connectNodes()
}

private func createNodes() {
    // 1. Create array of nodes
}

private func connectNodes() {
    // 2. Go through array, connecting each node to its neighbors 
    //    according to some predefined constants
}

Excited to hear any thoughts.

Answer 1

The problem you're dealing with is called temporal coupling

You're right to be concerned about how understandable this code is:

private func setupNodes() {
    createNodes();
    connectNodes();
}

I can guess what's going on there but tell me if this makes what else is going on a little clearer:

private func setupNodes() {
    self.nodes = connectNodes( createNodes() );
}

This has the added benefit of being less coupled to modifying instance variables but to me being readable is number one.

This makes connectNodes()'s dependency on nodes explicit.

Answer 2

Separate functions, for two reasons:

1. Private functions are private for exactly this situation.

Your init function is public, and it's interface, behavior, and return value is what you need to worry about protecting and changing. The result that you expect out of that method is going to be the same no matter what implementation you use.

Since the rest of the functionality is hidden behind that private keyword, it can be implemented however you like... so you might as well make it nice and modular, even if one bit depends on the other being called first.

2. Connecting nodes to each other might not be a private function

What if at some point you want to add other nodes to the array? Do you destroy the setup that you have now, and re-initialize it completely? Or do you add nodes to the existing array and then run connectNodes again?

Possibly connectNodes can have a sane response if the array of nodes hasn't been created yet (throw an exception? return an empty set? you have to decide what makes sense for your situation).

Answer 3

You may also find (depending on how complex each of these tasks are) that this is a good seam for splitting out another class.

(Not sure if Swift works this way but pseudo-code:)

class YourClass {
    init(generator: NodesGenerator) {
        self.nodes = connectNodes(generator.make())
    }
    private func connectNodes() {

    }
}

class NodesGenerator {
    public func make() {
        // Return some nodes from storage or make new ones
    }
}

This separates the responsibilities of creating and modifying nodes to separate classes:NodeGenerator only cares about creating/retrieving nodes, while YourClass only cares about connecting the nodes it is given.

Answer 4

In addition to this being the exact purpose of private methods, Swift gives you the ability to use inner functions.

Inner methods are perfect for functions that have only a single call site, but feel like they don't justify being separate private functions.

For example, it's very common to have a public recursive "entry" function, which checks preconditions, sets up some parameters, and delegates to a private recursive function which does the work.

Here's an example of how that might look in this case:

init() {
    self.nodes = setupNodes()

    func setupNodes() {
        var nodes = createNodes()
        connect(Nodes: nodes)
    }

    private func createNodes() -> [Node]{
        // 1. Create array of nodes
    }

    func connect(Nodes: [Node]) {
        // 2. Go through array, connecting each node to its neighbors 
        //    according to some predefined constants
    }
}

Pay attention to how I use return values and parameters to pass around data, rather than mutating a shared state. This makes the data flow much more obvious at first glance, without needing to jump into the implementation.

Answer 5

Every function you declare carries with it the burden of adding documentation and making it generalized so that it is usable by other parts of the program. It also carries the burden of understanding how other functions in the file may be using it for someone reading the code.

If however it isn't used by other parts of your program, I wouldn't expose it as a separate function.

If your language supports it, you can still have one-function-does-one-thing by using nested functions

function setupNodes ()  {
  function createNodes ()  {...} 
  function connectNodes ()  {...}
  createNodes() 
  connectNodes() 
} 

The place of declaration matters very much, and in the above example it is clear without needing any further clues that the inner functions are meant to be used only within the body of the outer function.

Even if you are declaring them as private functions, I assume that they are still visible to the whole file. So you would need to declare them close to the declaration of the main function, and add some documentation which clarifies that they are to be used only by the outer function.

I don't think that you have to strictly do one or the other. The best thing to do varies on a case by case basis.

Breaking it up into multiple functions certainly adds an overhead of understanding why there are 3 functions and how they all work with each other, but if the logic is complex then this added overhead may be much less than the simplicity introduced by breaking up the complex logic into simpler parts.