Simple Procedure, how can I reduce lines of Code

[Zabman]

Hi,

I have the following code, and I am certain it could be incorporated into a select case structure but can't get it to work. Can anyone offer any suggestions?

                If Range("Report_RevCli").Offset(StartCount) <> "Not found" Then
                    Range("Report_RevCli").Offset(StartCount).Font.Color = RGB(0, 0, 0)
                Else
                    Range("Report_RevCli").Offset(StartCount).Font.Color = RGB(255, 0, 0)
                End If
                If Range("Report_BillTo").Offset(StartCount) <> "Not found" Then
                    Range("Report_BillTo").Offset(StartCount).Font.Color = RGB(0, 0, 0)
                Else
                    Range("Report_BillTo").Offset(StartCount).Font.Color = RGB(255, 0, 0)
                End If
                If Range("Report_Customs").Offset(StartCount) <> "Not found" Then
                    Range("Report_Customs").Offset(StartCount).Font.Color = RGB(0, 0, 0)
                Else
                    Range("Report_Customs").Offset(StartCount).Font.Color = RGB(255, 0, 0)
                End If
                If Range("Report_DLV").Offset(StartCount) <> "Not found" Then
                    Range("Report_DLV").Offset(StartCount).Font.Color = RGB(0, 0, 0)
                Else
                    Range("Report_DLV").Offset(StartCount).Font.Color = RGB(255, 0, 0)
                End If
                If Range("Report_CustCat").Offset(StartCount) <> "Not found" Then
                    Range("Report_CustCat").Offset(StartCount).Font.Color = RGB(0, 0, 0)
                Else
                    Range("Report_CustCat").Offset(StartCount).Font.Color = RGB(255, 0, 0)
                End If

 

[Ruddles]

You're testing five conditions and choosing five out of ten actions. I can't see how you could make this any crunchier.

You can't use SELECT because that chooses one out of a number of conditions whereas you want to make choice based on each of the five conditions independently.

You could store the range names in a collection and loop through that, but it would look contrived and maybe not that easy to understand.

Just put a blank line between each IF block and at least it'll look nicer. Or make it a separate procedure and call it from your main procedure - that way you won't have to look at it!

 

[mvptomlinson]

The following is a bit cleaner. As Ruddles said, looping through the various ranges may look tidier. If only one of those five ranges can be "Not found" at a time, you could probably just do a Range.Find to look for "Not found" in those cells and change that font color.

With Range("Report_RevCli").Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With
                
With Range("Report_BillTo").Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With

With Range("Report_Customs").Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With

With Range("Report_DLV").Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With

With Range("Report_CustCat").Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With

 

[Zabman]

Thanks for the responses!

Unfortunately, any of the 5 fields may be listed as not found. Its part of a bigger loop looking at an index and any of the fields can be not found.

 

[mvptomlinson]

Perhaps this is a bit tidier, using that array mentioned earlier..

Sub zabman()
Dim arr(1 To 5) As String, i As Long, StartCount As Long
arr(1) = "Report_RevCli": arr(2) = "Report_BillTo": arr(3) = "Report_Customs"
arr(4) = "Report_DLV": arr(5) = "Report_CustCat"
StartCount = 1

For i = 1 To 5
With Range(arr(i)).Offset(StartCount)
    .Font.Color = RGB(IIf(.Value <> "Not found", 0, 255), 0, 0)
End With
Next i
End Sub

I set StartCount to 1 just as an example. That line could be removed if you're getting StartCount from elsewhere in the code.