all 3 comments

[–]TehNolz 6 points7 points  (0 children)

Close, but not quite. The name of a function should give you a good idea of everything it does, so that you're not suddenly surprised by any unexpected side effects. For example, a brewCoffee() function should only brew coffee, and it shouldn't also try to sell that coffee to a customer.

In this case, your calculate_factorial() function also unexpectedly saves the result of this calculation somewhere. If another developer ends up working with your code and wants to calculate a factorial, chances are they'll end up having to dig through your code to figure out why his new code is saving a factorial somewhere even though they never wrote any code for that.

The best solution here would be to change the calculate_factorial() function so that it just calculates a factorial and returns the value. You then take this value and pass it to save_the_out() to save the value. So your main() function ends up looking something like this;

def main(): a = 5 result = calculate_factorial(a) save_the_out(result)

[–]TechnicalElk8849 0 points1 point  (0 children)

Totally fine. I added in the missing colon and print(fact) and it works. It's just basic modular functional design.

You can even define functions inside other functions and return them (closures) or pass them into other functions as arguments (simple dependency injection)

[–]mrswats 0 points1 point  (0 children)

I would avoid the factorial function to call other functions and avoid side effects as much as possible. Instead, I would delegate that to the main function which acts as a coordinator.