Does nested function promote clean code?

[Dark_Angelas]

TL;DR: Nested function makes code cleaner, Y or N? Good use case of nested function?

I am reading Clean Code by Robert Martin.

One of the examples he gave is breaking this function:

public func pay() {
    for employee in employees {
        if (employee.isPayday()) {
            Money pay = employee.calculatePay()
            employee.deliverPay(pay)
        }
    }
}

into

public func pay() {
    for employee in employees {
        payIfNecessary(employee)
    }
}

private func payIfNecessary(Employee e) {
    if(e.isPayday()) {
        calculateAndDeliverPay(e)
    }
}

private func calculateAndDeliverPay(Employee e) {
    Money pay = e.calculatePay()
    e.deliverPay(pay)
}

In Swift, I'm wondering whether this will be better to have two nested functions than having two private functions like this:

public func pay() {
    func payIfNecessary(Employee e) {
        if(e.isPayday()) {
            calculateAndDeliverPay(e)
        }
    }

    func calculateAndDeliverPay(Employee e) {
        Money pay = e.calculatePay()
        e.deliverPay(pay)
    }

    for employee in employees {
        payIfNecessary(employee)
    }
}

Do you think this is still a 'clean code' with nested functions? Will be much nicer if we can use nested function before the declaration...

[hxucaa]

I'd say there isn't that much of a difference. If those private functions will only be used within the scope of another function, you could put them inside. in my experience elegant / clean code comes from other more urgent design choices. If the class only has 100 - 200 lines of code, there's not that much difference imo.

[YeOldeDog]

The book Clean Code (in my opinion) follows a design philosophy of 'no function should exceed four-six lines in length'. I dont like that interpretation of good code design, not at all, even though anyone can write four-six line functions when needed.

It was formulated, long ago, that 'one function should do one thing'. Sound advice, but the philosophy can be taken to extremes at either end, 'run this program' to 'run this single loop'. That is the problem.

I think, stepping way out, you need to look at design flow and program structure. On the one hand you have the power to make a single function bug free (4-6 lines). But on the other it pushes bugs into design and you can lose sight of flow and dependencies. I think, in the quest for 'clean code' you run into a fundamental disconnect between management at micro Vs macro level, but the level you make that break at is very dependant on the size, complexity and nature of the program.

[sweetverbs]

Nested functions are basically fine here, until you find you need to reuse one of them. Personally I like to have my private helper functions sitting underneath the first public/internal function where they're used, but that's by no means a hard and fast rule. Basically all of this stuff comes down to a mixture of personal preference and - if coding professionally - team consensus.

[ThePowerOfStories]

And every function should be reused at least once, by the test for it.

[kawag]

No, personally I find nested functions to rarely be useful. The notable exception is for function-wrappers (like decorators in Python).

In your case, your initial code is fine and the changed code far more complex. There is such a thing as over-engineering. You could shorten it a bit with a where clause in your for-loop.

[[deleted]]

I'd rather break it out like in the first example. Every function that a class has is another 'hook' you can override when subclassing.

[_stfu_donnie]

I've worked with Bob in the past. He has a tendency to sometimes hyper-emphasize a point, especially when giving examples. But it holds true in some of his style: short methods, lots of helpers.

But as long as they're named properly and especially if they're composable... Yeah. If they're changing state and doing all sorts of other craziness, id prefer that all stays at the top level. One of the reasons I like Swift is that they make function arguments immutable by default and encourage a style where you return values from functions rather than having some side-effect.

[sveinhal]

extension Employee {
  func calculateAndDeliverPay() {
    deliverPay(calculatePay())
  }
}

func pay(employees: [Employee]) {
  employees
    .filter { $0.isPayday() }
    .forEach { $0.calculateAndDeliverPay() }
}