3
\$\begingroup\$

This is my first Array in VBA for Excel, but I need some help to optimize the code and try to reduce the number of If statements.

The long and short of the code is that it checks to see if there is a Customer name in Column B and checks that against Column A; if there is no value in Column A then an input box opens to prompt the user to add the CIF number to the specific named range.

Cells B2 through B9 will always have a value, but not every cell B2 through B9 will be used. Cells A2 through A9 will not always have a value.

Below is the Code:

Sub CheckCIF()
    Dim fileArray
    Dim finalRow As Long
    Dim targetCol As Long


    With Sheets("Loan Data")
        finalRow = .Range("B" & Rows.Count).End(xlUp).Row
        targetCol = .Range("A1").EntireRow.Find("CIF #").Column
        fileArray = Array(.Range("B2:B" & finalRow), _
                    .Range(.Cells(2, targetCol), .Cells(finalRow, targetCol)))
    End With

    'fileArray(1)(1) Number in first parenthesis is the column and the number in the second parenthesis is the row

    If fileArray(1)(1) = vbNullString And fileArray(0)(1) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_1") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(1))
        End With
    End If

    If fileArray(1)(2) = vbNullString And fileArray(0)(2) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_2") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(2))
        End With
    End If

    If fileArray(1)(3) = vbNullString And fileArray(0)(3) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_3") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(3))
        End With
    End If

    If fileArray(1)(4) = vbNullString And fileArray(0)(4) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_4") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(4))
        End With
    End If

    If fileArray(1)(5) = vbNullString And fileArray(0)(5) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_5") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(5))
        End With
    End If

    If fileArray(1)(6) = vbNullString And fileArray(0)(6) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_6") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(6))
        End With
    End If

    If fileArray(1)(7) = vbNullString And fileArray(0)(7) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_7") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(7))
        End With
    End If

    If fileArray(1)(8) = vbNullString And fileArray(0)(8) <> vbNullString Then
        With Sheets("Loan Data")
            .Range("CIF_8") = InputBox("Please enter the CIF Number for " & vbCrLf & fileArray(0)(8))
        End With
    End If

End Sub
\$\endgroup\$
  • \$\begingroup\$ Does this code work as expected? Column A appears to be excluded from the array. \$\endgroup\$ – QHarr Mar 14 at 18:42
  • \$\begingroup\$ Yes the code works as expected. I reference Column A in this line targetCol = .Range("A1").EntireRow.Find("CIF #").Column and then in the fileArray(1)(1) where the 1 in the first () is column A. \$\endgroup\$ – Zack E Mar 14 at 18:46
  • \$\begingroup\$ But doesn't fileArray start from B? \$\endgroup\$ – QHarr Mar 14 at 18:49
  • \$\begingroup\$ Yes. I know its backwards, but it is my first Array ever and I get the expected result. \$\endgroup\$ – Zack E Mar 14 at 18:51
  • 1
    \$\begingroup\$ I will play around with it and see what I come up with. Thanks for looking at this for me. \$\endgroup\$ – Zack E Mar 14 at 19:00
2
\$\begingroup\$

The fileArray variable can be replaced with 2 variables. When looking at fileArray in the locals window, from the IDE menu at the top View>Locals Window, you can see that it's it a Variant/Variant(0 to 1). Expanding that by clicking on the + icon to the left of the variable name it it contains two sub arrays. fileArray(0) and fileArray(1) are both of type Variant/Object/Range. These are distinct areas that should be named respectively. customerArea for those in column B and cifArea for those in the same column as targetCol let you refer to these ranges directly.

Because these variables house information stored vertically, that is X rows tall by 1 column wide, you can reference the rows by customerArea(checkRow).Value2. Doing this allows you to write self documenting code. Rather than

If fileArray(1)(1) = vbNullString And fileArray(0)(1) <> vbNullString Then

You have

If customerArea(checkRow).Value2 = vbNullString And cifArea(checkRow).Value2 <> vbNullString Then

I'm torn between how to go about checking each of the rows. Mulling it over I decided on extracting the check into a Sub. I added a guard clause, endRow > customerArea.Rows.Count Or endRow > cifArea.Rows.Count, that raises an error if the end row exceeds the number of rows in each area that's checked. You could also add a guard clauses to ensure that startRow >= 1, or ensure that customerArea.Rows.Count = cifArea.Rows.Count or any others you deem appropriate, but that I'll leave for you.

Instead of checking each row explicitly I condensed that with a For...Next statement and used a descriptive variable checkRow to let you know which row you're checking.

Private Sub CheckForEmptyCellsIn(ByVal customerArea As Range, ByVal cifArea As Range, ByVal startRow As Long, ByVal endRow As Long)
    Const InputMessage As String = "Please enter the CIF Number for "

    Const InvalidArgument As Long = 5
    If endRow > customerArea.Rows.Count Or endRow > cifArea.Rows.Count Then
        Err.Raise InvalidArgument, "CheckForEmptyCells Sub", "endRow argument exceeded number of rows in"
    End If

    Dim checkRow As Long
    For checkRow = startRow To endRow
        If customerArea(checkRow).Value2 = vbNullString And cifArea(checkRow).Value2 <> vbNullString Then
            customerArea.Parent.Range("CIF_" & checkRow).Value2 = InputBox(InputMessage & vbCrLf & cifArea(1).Value2)
        End If
    Next
End Sub

This refactoring leaves you with

Public Sub CodeReview()
    Dim loanData As Worksheet
    Set loanData = Worksheets("Loan Data")
    Dim finalRow As Long
    finalRow = loanData.Range("B" & Rows.Count).End(xlUp).Row
    Dim targetCol As Long
    targetCol = loanData.Range("A1").EntireRow.Find("CIF #").Column

    CheckForEmptyCellsIn loanData.Range("B2:B" & finalRow), _
                        loanData.Range(loanData.Cells(2, targetCol), loanData.Cells(finalRow, targetCol)), _
                        1, 8
End Sub
\$\endgroup\$
  • \$\begingroup\$ Thank you for this. I did make a change so it would reference the Customer Name and not a specific row. Changed If customerArea(checkRow).Value2 = vbNullString And cifArea(checkRow).Value2 <> vbNullString Then customerArea.Parent.Range("CIF_" & checkRow).Value2 = InputBox(InputMessage & vbCrLf & cifArea(1).Value2) End If to this If customerArea(checkRow).Value <> vbNullString And cifArea(checkRow).Value = vbNullString Then cifArea.Parent.Range("CIF_" & checkRow).Value = InputBox(InputMessage & vbCrLf & customerArea(checkRow).Value) End If \$\endgroup\$ – Zack E Mar 18 at 13:35

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.