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
asked Mar 14, 2019 at 17:13
\$\endgroup\$
8
  • \$\begingroup\$ Does this code work as expected? Column A appears to be excluded from the array. \$\endgroup\$ Commented Mar 14, 2019 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\$ Commented Mar 14, 2019 at 18:46
  • \$\begingroup\$ But doesn't fileArray start from B? \$\endgroup\$ Commented Mar 14, 2019 at 18:49
  • \$\begingroup\$ Yes. I know its backwards, but it is my first Array ever and I get the expected result. \$\endgroup\$ Commented Mar 14, 2019 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\$ Commented Mar 14, 2019 at 19:00

1 Answer 1

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
answered Mar 18, 2019 at 4:28
\$\endgroup\$
1
  • \$\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\$ Commented Mar 18, 2019 at 13:35

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.