Report Building (Data Retrieval, Validation, Aggregation, Business Logic, Report Building, Visual Presentation)

This:

Is a data table we get from our financial platform with lots of useful information. For reference, "--" is also the string they use to denote empty values.

This:

Is a spreadsheet I built (and had reviewed here) to track our customers' regular income requirements.

The code produces reports to show:

-
Whether clients with upcoming income have sufficient cash available in their accounts

-
Which accounts have a large % uninvested (above a minimum threshold value)

-
Which accounts are not attached to an investment model

-
A table of account notes appended to each of the above

Overview of the code:

Step 1: Get raw data into arrays

Step 2: Clean said data

Step 3: Produce Reports

Step 4: Print Reports to sheets and Format Presentation

N.B. The worksheet variables are codenames

N.B. There are a number of what I'll call "standard Functions" that appear in the code. You may safely assume that they do what they say they do.

As always, open to any and all feedback, at any level of abstraction. That said, I'm especially interested in the larger project-structure and general maintainability.

Module B1_Public_Variables

```
Option Explicit

Public Const WB_INCOME_LIST_FILEPATH As String = "S:\Lumin Admin Docs\Ascentric Cash Management\"
Public Const WB_INCOME_LIST_FILENAME As String = "Ascentric Client Income List.xlsm"

Public Const ASCENTRIC_TOP_LEFT_CELL_STRING As String = "Adviser" '/ At present, on row 3
Public Const NOTES_TOP_LEFT_CELL_STRING As String = "Adviser"

'/ Headers for this workbook

Public Const ADVISER_NAME_HEADER As String = "Adviser"
Public Const CLIENT_NAME_HEADER As String = "Client Name"
Public Const ASCENTRIC_NUMBER_HEADER As String = "Client Ref"
Public Const PRODUCT_CODE_HEADER As String = "Product"
Public Const WRAPPER_VALUE_HEADER As String = "Wrapper Value (WV)"
Public Const INVESTMENT_MODEL_HEADER As String = "Model Name"
Public Const DEPOSIT_ACCOUNT_HEADER As String = "Deposit Cash"
Public Const RESERVE_ACCOUNT_HEADER

I think you should work on cleaning the small bits of your code through the use of small functions.

If sippAmount > regIncomeAmount Then
        incomeAmount = sippAmount
    Else
        incomeAmount = regIncomeAmount
    End If

Like here, I don't know what the VBA call is, but isn't there some version of MAX that you could use?

Or here,

Public Function InitialiseIncomeReportColumnNumbers() As Scripting.Dictionary

    Dim dict As Scripting.Dictionary
    Set dict = New Scripting.Dictionary

    Dim header As String, colNum As Long

    header = ADVISER_NAME_HEADER
    colNum = 1
    dict.Add header, colNum

    header = CLIENT_NAME_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = ASCENTRIC_NUMBER_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = PRODUCT_CODE_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = RESERVE_ACCOUNT_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = DEPOSIT_ACCOUNT_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = INCOME_ACCOUNT_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = TRADING_ACCOUNT_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = ACCOUNT_TO_TAKE_INCOME_FROM_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = SIPP_INCOME_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = REGULAR_INCOME_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = INCOME_FREQUENCY_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    header = NEXT_INCOME_DATE_HEADER
    colNum = colNum + 1
    dict.Add header, colNum

    Set InitialiseIncomeReportColumnNumbers = dict

End Function

Can't you make a separate addAll function that takes a bunch of values (all your headers) and handles the colNum crap for you?

Or this one, where you're replacing some specific strings in CleanUpAscentricData

valueToReplace = "AGENERAL"
replacementValue = "GIA"
ascentricDataArray = FindAndReplace2DArrayValues(ascentricDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL2"
replacementValue = "GIA2"
ascentricDataArray = FindAndReplace2DArrayValues(ascentricDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL3"
replacementValue = "GIA3"
ascentricDataArray = FindAndReplace2DArrayValues(ascentricDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL4"
replacementValue = "GIA4"
ascentricDataArray = FindAndReplace2DArrayValues(ascentricDataArray, valueToReplace, replacementValue)

and CleanUpNotesData:

valueToReplace = "AGENERAL"
replacementValue = "GIA"
notesDataArray = FindAndReplace2DArrayValues(notesDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL2"
replacementValue = "GIA2"
notesDataArray = FindAndReplace2DArrayValues(notesDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL3"
replacementValue = "GIA3"
notesDataArray = FindAndReplace2DArrayValues(notesDataArray, valueToReplace, replacementValue)

valueToReplace = "AGENERAL4"
replacementValue = "GIA4"
notesDataArray = FindAndReplace2DArrayValues(notesDataArray, valueToReplace, replacementValue)

Looks like a good place for a generic helper function to me. Array + a mapping of "from, to" goes in, array with replaced values comes out.

Here's another one...

Dim ws As Worksheet
Dim arr As Variant

Set ws = wsClientIncomeReport
arr = incomeReportArray
ws.Cells.Clear
Print2dArrayToSheet wbUninvestedCash, ws, arr, ws.Cells(1, 1)
FormatIncomeReportVisuals arr, ws

Set ws = wsRemovedIncome
arr = removedIncomeRows
ws.Cells.Clear
Print2dArrayToSheet wbUninvestedCash, ws, arr, ws.Cells(1, 1)
FormatIncomeReportVisuals arr, ws

Set ws = wsNoModelAttachedreport
arr = noModelArray
ws.Cells.Clear
Print2dArrayToSheet wbUninvestedCash, ws, arr, ws.Cells(1, 1)
FormatNoModelReportVisuals arr, ws

Set ws = wsUninvestedCashReport
arr = uninvestedCashReport
ws.Cells.Clear
Print2dArrayToSheet wbUninvestedCash, ws, arr, ws.Cells(1, 1)
FormatUninvestedCashReportVisuals arr, ws

You're doing something that looks very much the same 4 times in a row. Would it be possible to just pass the values into a function?

Or there's here

If header = ADVISER_NAME_HEADER Then
                If hasAdviser Then
                    value = dataArray(ix, sourceColNum)
                    newArr(iy, currentRow) = value
                End If
            Else
                value = dataArray(ix, sourceColNum)
                newArr(iy, currentRow) = value
            End If

Where you do something if (A && B) || !A... if we make a truth table out of that, we can see that...

a: header = ADVISER_NAME_HEADER 
b: hasAdviser

a | b | a && b | !a | (a && b) or !a
====================================
0 | 0 |   0    | 1  | 1
0 | 1 |   0    | 1  | 1
1 | 0 |   0    | 0  | 0
1 | 1 |   1    | 0  | 1

Only the case (a and n

StackExchange Code Review Q#116923, answer score: 7