11
\$\begingroup\$

I generate a set of data based off every combination of inputs. The source range that's inputs requires headers to identify the parameters. In its present state I'm not content as it's not as straightforward as I'd like it to be. Specifically the module variables can be cleaned up but I'm not seeing how.

A simple example with 2 columns

---------------------
| Column1 | Column2 |
|---------|---------|
|    A    |    1    |
|    B    |    2    |
|    C    |    3    |
---------------------

generates

---------------------
| Column1 | Column2 |
|---------|---------|
|    A    |    1    |
|    A    |    2    |
|    A    |    3    |
|    B    |    1    |
|    B    |    2    |
|    B    |    3    |
|    C    |    1    |
|    C    |    2    |
|    C    |    3    |
---------------------

Adding a 3 column input table

-------------------------------
| Column1 | Column2 | Column3 |
|---------|---------|---------|
|    A    |    1    |   F1    |
|    B    |    2    |   F2    |
|    C    |    3    |   F3    |
|         |         |   F4    |
-------------------------------

generates

-------------------------------
| Column1 | Column2 | Column3 |
|---------|---------|---------|
|    A    |    1    |   F1    |
|    A    |    1    |   F2    |
|    A    |    1    |   F3    |
|    A    |    1    |   F4    |
|    A    |    2    |   F1    |
 . . . . . . . . . . . . . . .
|    C    |    2    |   F4    |
|    C    |    3    |   F1    |
|    C    |    3    |   F2    |
|    C    |    3    |   F3    |
|    C    |    3    |   F4    |
-------------------------------

and so on.

The class module InputColumn is used to help compartmentalize the logic for each column and its corresponding max depth being equivalent to CountOfValues.

'@PredeclaredId
'@Folder("ParametricSweep.Model")
Option Explicit

Private Type THelper
    ColumnIndex As Long
    Values As Variant
    Identifier As String
End Type

Private this As THelper

Public Property Let Values(ByVal Value As Variant)
    this.Values = Value
End Property
Public Property Get Values() As Variant
    Values = this.Values
End Property

Public Property Get CountOfValues() As Long
    CountOfValues = UBound(Values) - LBound(Values) + 1
End Property

Public Property Let Identifier(ByVal Value As String)
    this.Identifier = Value
End Property
Public Property Get Identifier() As String
    Identifier = this.Identifier
End Property

Public Function Self() As InputColumn
    Set Self = Me
End Function

Public Function Create(ByVal ColumnIndex As Long, ByVal sourceArea As Range) As InputColumn
    Dim populatedCells As Range
    Set populatedCells = sourceArea.SpecialCells(XlCellType.xlCellTypeConstants)

    With New InputColumn
        .Identifier = populatedCells(1, 1).Value2
        .Values = Application.WorksheetFunction.Transpose(populatedCells.Offset(1).Resize(populatedCells.Rows.Count - 1).Value2)

        Set Create = .Self
    End With
End Function

The sweep of values is generated by populating the inputColumns() array then stepping into PopulateArray 1 for the first (topmost) level. After the call to PopulateArray it recursively descends one level deeper until it's populating the last column. As each loop returns it increments the array recursionInputColumnPopulationIndex() that identifies which element should populated for the corresponding recursion depth.

Option Explicit

Private parentArray() As Variant
Private inputColumns() As InputColumn
Private maxRecursionDepth As Long
Private recursionInputColumnPopulationIndex() As Long
Private populationRow As Long

Public Sub TestRecursive()
    Dim foo As Variant
    foo = RecursiveParametricSweep(Sheet1.Range("C5").CurrentRegion, True)

    Dim bar As Variant
    bar = RecursiveParametricSweep(Sheet1.Range("C16").CurrentRegion, False)
End Sub

Public Function RecursiveParametricSweep(ByVal inputSourceArea As Range, ByVal includeHeader As Boolean) As Variant
    maxRecursionDepth = inputSourceArea.Columns.Count

    ReDim inputColumns(1 To maxRecursionDepth)
    ReDim recursionInputColumnPopulationIndex(1 To maxRecursionDepth)

    PopulateInputColumns inputSourceArea

    Dim rowCount As Long
    rowCount = GetRowCount(inputColumns)
    If includeHeader Then
        ReDim parentArray(0 To rowCount, 1 To maxRecursionDepth)

        Dim headerRow As Long
        headerRow = LBound(parentArray)
        Dim headerColumn As Long
        For headerColumn = LBound(inputColumns) To UBound(inputColumns)
            parentArray(headerRow, headerColumn) = inputColumns(headerColumn).Identifier
        Next

        populationRow = LBound(parentArray) + 1
    Else
        ReDim parentArray(1 To rowCount, 1 To maxRecursionDepth)
        populationRow = LBound(parentArray)
    End If

    PopulateArray 1, includeHeader
    RecursiveParametricSweep = parentArray
End Function

Private Sub PopulateArray(ByVal recursionDepth As Long, ByVal includeHeader As Boolean)
    Dim populateElementCount As Long
    For populateElementCount = 1 To inputColumns(recursionDepth).CountOfValues
        If recursionDepth < maxRecursionDepth Then
            PopulateArray recursionDepth + 1, includeHeader

            recursionInputColumnPopulationIndex(recursionDepth) = recursionInputColumnPopulationIndex(recursionDepth) + 1
        Else
            Dim columnPopulationIndex As Long
            For columnPopulationIndex = recursionDepth To 1 Step -1
                parentArray(populationRow, columnPopulationIndex) = inputColumns(columnPopulationIndex).Values(recursionInputColumnPopulationIndex(columnPopulationIndex) + 1)
            Next

            recursionInputColumnPopulationIndex(recursionDepth) = recursionInputColumnPopulationIndex(recursionDepth) + 1
            populationRow = populationRow + 1
        End If
    Next
    recursionInputColumnPopulationIndex(recursionDepth) = 0
End Sub

Private Function GetRowCount(ByRef inputColumns() As InputColumn) As Long
    GetRowCount = 1
    Dim depthCounter As Long
    For depthCounter = LBound(inputColumns) To UBound(inputColumns)
        GetRowCount = GetRowCount * inputColumns(depthCounter).CountOfValues
    Next
End Function

Private Sub PopulateInputColumns(ByVal inputSourceArea As Range)
    Dim populateInputColumnsCounter As Long
    For populateInputColumnsCounter = LBound(inputColumns) To UBound(inputColumns)
        Set inputColumns(populateInputColumnsCounter) = InputColumn.Create(populateInputColumnsCounter, inputSourceArea.Columns(populateInputColumnsCounter))
    Next
End Sub
```
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

This looks like a decent recursive implementation. So, this is mainly about style and readability.

First, I would like to talk about two things which are really hard and could use some improvement here, naming and consistency of semantic levels. Then I have some more miscellaneous comments.

Naming

Generally, the naming is not too bad, but some of the names are a bit misleading, e.g. the function name GetRowCount suggests that it returns the number of rows of whatever I give it. However, what it returns is the number of rows of the later output. So, one could simply call it OutputRowCount, which also follows the general guideline to use nouns that describe the return value for functions.

Another things with the naming is that it sticks a bit much to the role of things in the implementation and not to what they are. Naming things after what they are can make understanding the code much faster. E.g. using inputColumnIndex instead of recursionDepth would immediately tell the reader that you are dealing with this specific input column right now. Then the technical maxRecursionDepth could also be lastColumnIndex or countOfColumns. Similarly, the rather ominous recursionInputColumnPopulationIndex(recursionDepth) could be currentInputIndex(inputColumnIndex). So generally, my advice would be to name things after what they are and not their implementation purpose.

Consistency of Semantic Levels

Programmers, me included, are notoriously bad a keeping semantic levels consistent in procedures. To explain what I mean take a look at RecursiveParametricSweep. It does some high level orchestration like calling the procedure to populate the input columns, calling a function to get the number of output rows, calling the procedure to populate the output array and assigning the return value. However, in between it goes into the detail of how to handle headers. This is a break in semantic level of the procedure that throws one off a bit when first reading the procdure. This could be avoided by extracting either an InitializeOutputArray or a WriteHeaders procedure.

Miscellaneous

  • The global variable maxRecursionDepth is a bit superfluous since it is just UBound(inputColumns), which is a bit clearer semantically, I think.
  • Instead of incrementing recursionInputColumnPopulationIndex(recursionDepth), you could just always set it to populateElementCount at the start of the loop, which might be called currentInputIndex instead.
  • Istead of keeping track of the indeces for the columns in an array, you might just as well keep track of the values in an array instead. That would put the choice of value closer to the action for the specific column.
  • It looks a bit strange to have the populationRow counter as a module level variable. You could avoid this by passing it ByRef into the recursive procedure.
  • It might make sense to extract a PopulateRow procedure to keep semantic levels consitsent.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.