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
```
Mathieu Guindon
75.5k18 gold badges194 silver badges467 bronze badges
asked Sep 28, 2019 at 5:48
\$\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.
answered Sep 28, 2019 at 8:55
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.