Coding Style – Functions That Only Call Other Functions

coding-style

I'm currently working on a set of reports that have many different sections (all requiring different formatting), and I'm trying to figure out the best way to structure my code. Similar reports we've done in the past end up having very large (200+ line) functions that do all of the data manipulation and formatting for the report, such that the workflow looks something like this:

DataTable reportTable = new DataTable();
void RunReport()
{
    reportTable = DataClass.getReportData();
    largeReportProcessingFunction();
    outputReportToUser();
}

I would like to be able to break these large functions up into smaller chunks, but I'm afraid that I'll just end up having dozens of non-reusable functions, and a similar "do everything here" function whose only job is to call all these smaller functions, like so:

void largeReportProcessingFunction()
{
    processSection1HeaderData();
    calculateSection1HeaderAverages();
    formatSection1HeaderDisplay();
    processSection1SummaryTableData();
    calculateSection1SummaryTableTotalRow();
    formatSection1SummaryTableDisplay();
    processSection1FooterData();
    getSection1FooterSummaryTotals();
    formatSection1FooterDisplay();

    processSection2HeaderData();
    calculateSection1HeaderAverages();
    formatSection1HeaderDisplay();
    calculateSection1HeaderAverages();
    ...
}

Or, if we go one step further:

void largeReportProcessingFunction()
{
    callAllSection1Functions();

    callAllSection2Functions();

    callAllSection3Functions();
    ...        
}

Is this really a better solution? From an organizational point of view I suppose it is (i.e. everything is much more organized than it might otherwise be), but as far as code readability I'm not sure (potentially large chains of functions that only call other functions).

Thoughts?

Best Answer

This is commonly referred to as functional decomposition and is generally a good thing to do, if done correctly.

Also, the implementation of a function should be within a single level of abstraction. If you take largeReportProcessingFunction, then its role is to define which different processing steps are to be taken in which order. The implementation of each of those steps is on an abstraction layer below and largeReportProcessingFunction shouldn't depend on it directly.

Please note that this however is a bad choice of naming:

void largeReportProcessingFunction() {
    callAllSection1Functions();
    callAllSection2Functions();
    callAllSection3Functions();
    ...        
}

You see callAllSection1Functions is a name that doesn't actually provide an abstraction, because it doesn't actually say what it does, but rather how it does it. It should be called processSection1 instead or whatever is actually meaningful in the context.

Related Topic