Passing same argument to multiple small functions versus doing everything in one big function?

Question

I have inherited a class method from another developer, which looks like below: (Note: Class:X means X is a member of Class in the OOP paradigm.)

Class:BigFunction() 
{
    ImportantValue = calculateImportantValue()

    Iterate over Class:Foo from 1 to ImportantValue
    Iterate over Class:Bar from 1 to ImportantValue
    Iterate over Class:Baz from 1 to ImportantValue

    # ... and so on ...
}

I am considering refactoring this to something like:

Class:WrapSeveralSmallFunctions() 
{
    ImportantValue = calculateImportantValue()

    IterateFoo(ImportantValue)
    IterateBar(ImportantValue)
    IterateBaz(ImportantValue)

    # ... and so on ...
}

Class:IterateFoo(ImportantValue)
{
    Iterate over Foo from 1 to ImportantValue
}

Class:IterateBar(ImportantValue)
{
    Iterate over Bar from 1 to ImportantValue
}

Class:IterateBaz(ImportantValue)
{
    Iterate over Baz from 1 to ImportantValue
}

Per Single Responsibility Principle, the refactored code looks better to me.

However, I am also concerned about passing the ImportantValue as an argument to every such "small" function. Is my concern valid? Is the proposed refactoring considered a good coding practice, or is there any better way of writing this code?

consider [extract class refactoring](http://programmers.stackexchange.com/a/111186/31260) — gnat, May 28 '15 at 04:42
surely you should do only one loop 0 to importantValue and do all three things in it — Ewan, May 28 '15 at 05:18

score 2 · Accepted Answer · answered May 28 '15 at 05:25

In term of performance, there will be virtually no difference between the two considered options.

Theoretically, the Single Responsibility Principle suggests that the solution with several small functions called by the big one is preferable if the small functions make sense. Are there semantically well defined? When you read the function names and the for-loops in the execution flow, do you read a consistent story?

If, taken individually, the small functions don't have a clear, well defined goal, your refactoring is probably too fine grained and counterproductive. Depending on your programming language, you can then consider to use advanced list comprehension or functional style coding for reducing verbosity.

For instance, the following code limits the handling of each collection to a single line, which is as simple to understand as a call to a small function with a for-loop.

Class:BigFunction()
{
    ImportantValue = calculateImportantValue()

    Class:Foo.take(ImportantValue).foreach(f => apply(f))
    Class:Bar.take(ImportantValue).foreach(f => apply(f))
    Class:Baz.take(ImportantValue).foreach(f => apply(f))
}

If you systematically have to process your three lists in multiple "big" functions, it can be an evidence that the class itself violates the SRP. You therefore should consider to split the class. This operation is also a refactoring.

Passing same argument to multiple small functions versus doing everything in one big function?

1 Answers1