2
\$\begingroup\$

This function is called by a sub that feeds it lines that the function needs to print for all the elements in an array.

This function retrieves a string, line_to_print that contains a substring replace_this_keyword_0 called "replace_this_keyword_0" (sometimes occuring twice in the same string) when it is called. It also receives a reference to the row number in which the array is listed in, in "Sheet1" in excel.

Next the number of used dimensions for that array is given by used_dimension(row_variables). As well as the starting index and ending index for that dimension of that array in start_index(dimension, row_variables) and end_index(dimension, row_variables).

Based on these input parameters, it loops through all the elements of the array and repeatedly prints out the line with the changed array indices until it has printed the line for all the array elements.

The purpose of this function is to print lines of code, that are copied in a larger procedure.

The purpose of the sub calling this function is to code allow a too large procedure to be refactored by printing a code that passes all the variables registered in the local window of the VBA editor of excel 2016 in a collection that is then easily and completely passed to multiple smaller pieces of code that are broken off of the too large code.

Initially I could not get the extract method of Rubberduck working. This led me to write this code. I see it as a learning exercise to increase my understanding of how variables are transferred between different types of code in VBA.

My main concern is the pattern that occurs in the nested for loops. I think it can be written more compact without sacrificing too much readability.

Function loop_through_indices(row_variables, used_dimension, start_index, end_index, line_to_print, replace_this_keyword_0, lastrow)

'0. This function absorbs the line that needs to be written with keyword: replace_this_keyword_0
'1. then it loops through all the indices of the array that the line to be printed contains, in row_variables
'2. and replaces the keyword with the indices string (E.g.: (4,3,9)
'3. then it writes the line by calling sub write_line(index_string, line_to_print, replace_this_keyword_0)

'This function requires the following data:
'a. row
'b. number of dimensions used by variable
'c. the starting index of each used dimension
'd. the ending index of each used dimension
'e. the line that needs to be printed
'f. the keyword

'a= row_variables
'b= used_dimension(row_variables)
'c= start_index(dimension_test, row_array)
'd= end_index(dimension_test, row_array)
'e= ..
'f= replace_this_keyword_0

    Dim index_1 As Integer
    Dim index_2 As Integer
    Dim index_3 As Integer
    Dim index_4 As Integer
    Dim index_5 As Integer
    Dim index_string As String

    For index_1 = start_index(1, row_variables) To end_index(1, row_variables)
        If used_dimension(row_variables) > 1 Then
            For index_2 = start_index(2, row_variables) To end_index(2, row_variables)
                If used_dimension(row_variables) > 2 Then
                    For index_3 = start_index(3, row_variables) To end_index(3, row_variables)
                        If used_dimension(row_variables) > 3 Then
                            For index_4 = start_index(4, row_variables) To end_index(4, row_variables)
                                If used_dimension(row_variables) > 4 Then
                                    For index_5 = start_index(5, row_variables) To end_index(5, row_variables)
                                        index_string = "(" & index_1 & "," & index_2 & "," & index_3 & "," & index_4 & "," & index_5 & ")"
                                        Call write_line(index_string, line_to_print, replace_this_keyword_0)
                                    Next index_5
                                Else
                                    index_string = "(" & index_1 & "," & index_2 & "," & index_3 & "," & index_4 & ")"
                                    Call write_line(index_string, line_to_print, replace_this_keyword_0)
                                End If
                            Next index_4
                        Else
                            index_string = "(" & index_1 & "," & index_2 & "," & index_3 & ")"
                            Call write_line(index_string, line_to_print, replace_this_keyword_0)
                        End If
                    Next index_3
                Else
                    index_string = "(" & index_1 & "," & index_2 & ")"
                    Call write_line(index_string, line_to_print, replace_this_keyword_0)
                End If
            Next index_2
        Else
            index_string = "(" & index_1 & ")"
            Call write_line(index_string, line_to_print, replace_this_keyword_0)
        End If
    Next index_1
End Function

For completeness, function loop_through_indices calls subroutine write_line to write the line.

Sub write_line(index_string, line_to_print, replace_this_keyword_0)
    Dim line_with_indices As String
    line_with_indices = Replace(line_to_print, replace_this_keyword_0, index_string)
    Print #1, line_with_indices
    'MsgBox ("line_with_indices =" & vbNewLine & line_with_indices)
End Sub

I initially tried to replace the list of index_.. strings with an array of strings, but there was a problem with the use of that array element as a counter. I am retrying that implementation now as I noticed the following code does work:

Function loop_through_indices(row_variables, used_dimension, start_index, end_index, line_to_print, replace_this_keyword_0, lastrow)

'0. This function absorbs the line that needs to be written with keyword: replace_this_keyword_0
'1. then it loops through all the indices of the array that the line to be printed contains, in row_variables
'2. and replaces the keyword with the indices string (E.g.: (4,3,9)
'3. then it writes the line by calling sub write_line(index_string, line_to_print, replace_this_keyword_0)

'This function requires the following data:
'a. row
'b. number of dimensions used by variable
'c. the starting index of each used dimension
'd. the ending index of each used dimension
'e. the line that needs to be printed
'f. the keyword

'a= row_variables
'b= used_dimension(row_variables)
'c= start_index(dimension_test, row_array)
'd= end_index(dimension_test, row_array)
'e= ..
'f= replace_this_keyword_0

    Dim index_1 As Integer
    Dim index_2 As Integer
    Dim index_3 As Integer
    Dim index_4 As Integer
    Dim index_5 As Integer
    Dim index_string As String
    Dim index(1) As Integer


    For index(1) = start_index(1, row_variables) To end_index(1, row_variables)
        If used_dimension(row_variables) > 1 Then
            For index_2 = start_index(2, row_variables) To end_index(2, row_variables)
                If used_dimension(row_variables) > 2 Then
                    For index_3 = start_index(3, row_variables) To end_index(3, row_variables)
                        If used_dimension(row_variables) > 3 Then
                            For index_4 = start_index(4, row_variables) To end_index(4, row_variables)
                                If used_dimension(row_variables) > 4 Then
                                    For index_5 = start_index(5, row_variables) To end_index(5, row_variables)
                                        index_string = "(" & index_1 & "," & index_2 & "," & index_3 & "," & index_4 & "," & index_5 & ")"
                                        Call write_line(index_string, line_to_print, replace_this_keyword_0)
                                    Next index_5
                                Else
                                    index_string = "(" & index_1 & "," & index_2 & "," & index_3 & "," & index_4 & ")"
                                    Call write_line(index_string, line_to_print, replace_this_keyword_0)
                                End If
                            Next index_4
                        Else
                            index_string = "(" & index_1 & "," & index_2 & "," & index_3 & ")"
                            Call write_line(index_string, line_to_print, replace_this_keyword_0)
                        End If
                    Next index_3
                Else
                    index_string = "(" & index_1 & "," & index_2 & ")"
                    Call write_line(index_string, line_to_print, replace_this_keyword_0)
                End If
            Next index_2
        Else
            index_string = "(" & index_1 & ")"
            Call write_line(index_string, line_to_print, replace_this_keyword_0)
        End If
    Next index(1)
End Function
\$\endgroup\$
  • \$\begingroup\$ @AJD Thank you for pointing that out. It is correct, but I formulated it ambiguously. So I clarified the statement to explicitly mention that the subroutine write_line is called by the function loop_through_indices. The function loop_through_indices in turn is called by another subroutine which is left out since it currently is not part of the focus. \$\endgroup\$ – a.t. Jun 3 '18 at 6:46
  • 2
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Simon Forsberg Jun 4 '18 at 20:45
4
\$\begingroup\$

Let's start with the "Simple" stuff.

Naming Conventions

In vba, underscores are used to designate inheritance and event handlers. Unless you're inheriting a function/sub or it's an event handler, you should avoid underscore to not suggest it's either of these special things.

Explicit Typing

You're already nicely declaring your local variables with an explicit type. You should do the same for function return values and parameters.

Also note that a function returns a value. If you do not return a value, you should use a Sub instead.

Do note that Integer is only a 16-bit signed Integer in VBA. To use the faster and more common 32-bit signed integer, you should declare your variables as Long

Obsolete Call

Call is deprecated. You do not need it. So leave it out :)

The algorithm

It looks like you're trying to iterate a multi-dimensional array, but you're missing a few useful things:

It's probably easier to read the data you have into a multi-dimensional array directly and iterate that array. This eliminates used_dimension, start_index and end_index in favor of recursion and the use of LBound and UBound.

Sub LoopIndices(variables As Variant, line As String, replaceThis As String, built As String = "(")
    Dim i As Long
    Dim v As Variant
    For i = LBound(variables) To UBound(variables)
        v = variables(i)
        If IsArray(v) Then
            LoopIndices(v, line, replaceThis, built & i & ", ")
        Else 
            WriteLine(built & i & ")", line, replaceThis)
        End If
    Next i
End Sub

This has the added benefit of dealing with nearly arbitrarily many dimensions. It's also rather clear in how it deals with variables

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the clear feedback! I will look into why a Long is faster than an Integer even though it takes up more bits. I was indeed unaware of the naming convention you mentioned and I will focus on applying a function and sub in the appropriate moments. The function return values I still have to look into a bit further. And the Algorithm suggestion will take me some time to digest. So I am very great full for your feedback! \$\endgroup\$ – a.t. Jun 5 '18 at 15:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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