VBA Excel IsError within For loop issues

Question

I get daily reports that have a bunch of grouped tables in several worksheets. There are only certain lines in each table I'm interested in. There is a subset within the value of a cell in each of those lines that determines if it's something I'm interested in or not. That value needs to match the value in a column I have in another sheet. Once I have a match then I can determine which color I want to apply to that line.

I've spent a lot of time on this already, and so far everything I've tried that I could get to run has gotten some of what I want to work, but not all of it. Once I get it working for the first table, I should be able to scale the macro to handle the rest of the workbook.

I started by defining MyList to be the range of cells I want to compare against from the other sheet. Then I set the other variables that I'll eventually be changing when I scale this up. Then I'm removing the existing conditional formatting from the whole workbook since I won't be needing that once I get this all working. Then I activate the worksheet that I want to be working on. The for loop seems to run fine. I have a watch set up for the NB variable, and I can see that update correctly for each loop. The problem I'm running into is that no matter what I do with the IsError(Match()) it never seems to update with each iteration of the for loop. So it either stays at false the entire time and colors every row in the table, or it stays true the entire time and colors nothing (depending on which arrangement of the code I try).

I done a bunch of google searching and reading and can't find the right information to figure out how to fix this. Am I trying to use something in a way that I shouldn't, or am I not defining something correctly? What else am I missing?

I'm new to VBA and haven't really done much of any coding for a number of years. So, in addition to fixing this I'm really interested in the why behind it so I can hopefully not repeat those mistakes.

Set MyList = Worksheets("Sheet1").Range("A1", "A500")
Dim NBcol As Integer, MZcol As Integer, blcol As Integer, NB As String
NBcol = 13
MZcol = 16
blcol = 12

Dim ws As Worksheet
For Each ws In ThisWorkbook.Worksheets
    ws.Cells.FormatConditions.Delete
Next ws

Worksheets("Sheet2").Activate
For i = 7 To 26
    NB = Left(Cells(i, NBcol), 6)
    If Not IsError(Application.Match(NB, MyList, 0)) Then
        If Cells(i, MZcol) >= 3.5 Then
            ActiveSheet.Range(Cells(i, blcol), Cells(i, MZcol)).Interior.Color = RGB(250, 191, 143)
        Else
            ActiveSheet.Range(Cells(i, blcol), Cells(i, MZcol)).Interior.Color = RGB(197, 217, 241)
        End If
    Else
    End If
Next i

in your outer `Else` put `MsgBox NB & " not found"` My guess is that is always errors, as it never finds the match. Remember that Match finds an exact match, type matters. Also if it is strings make sure there are not spaces or other characters that cause it not to be a match. — Scott Craner, Apr 06 '20 at 18:25
@SJR I have not tried using F8. I've been using watches, and then I added breakpoints at certain points within the loop, so I could see where things were going and check the watches along the way. What other formatting are you referring to that I should be clearing, and how/where in here should I be doing that? — JSP, Apr 06 '20 at 18:34
@ScottCraner It pops the message box up on every iteration as I have it right now. I was thinking it was possibly a datatype match issues. How do I determine what datatypes I need to make it work and get them in that format? At some point I tried wrapping the NB = Left ... within a Cint() but all that ended up doing is giving me errors that prevent it from running. I was an overflow IIRC. — JSP, Apr 06 '20 at 18:39
Change your definition of `NB` to `NB As Long` instead of `String`. You're forcing it to compare strings when the data is actually a number. — PeterT, Apr 06 '20 at 18:51
@SJR the range on Sheet1 is all 6 digit numbers. Excel shows them as the general format. NB is taking the 6 characters on the left of the cell in Sheet2 M7 (first loop iteration), which is ######_#_# with varying degrees of underscores/digits afterwards. — JSP, Apr 06 '20 at 18:51
@PeterT Thanks. That seems to have cleared up my current issue. Is there a really good guidance somewhere to make it easy to know which datatype is needed for a given function? — JSP, Apr 06 '20 at 18:58
Whole numbers --> `...As Long`, Fractional numbers --> `...As Double` — Tim Williams, Apr 06 '20 at 19:01
You were implicitly casting data types in your `NB = Left(Cells(i, NBcol), 6)` statement, plus a few other things that may trip you later. First, fully qualify ***ALL*** of your [worksheet references](https://www.spreadsheetsmadeeasy.com/7-common-vba-mistakes-to-avoid/) and don't use `Select` or `Activate`. Next, your `Cells(i, NBcol)` is implicitly using the `.Value` of the cell (in your case it's a number), then it implicitly converts that to a `String` when you feed it to the `Left` function. The `Match` function can use anything, as long as the input and search range types match. — PeterT, Apr 06 '20 at 19:07
@PeterT I see some of the things I have done called out in that link you included. The Cells(i, NBcol) should just be feeding the cell reference to the Left function. Can you expand on what you mean by implicitly using the .Value? What else could it be doing? so Left defaults to output as a String? Then because we set NB as long it gets converted back to a number for comparison? I see how .Activate for a sheet could be problematic now. I have 3 sheets of these tables, that I could run the same bit of code on if I looped it through activating each sheet in turn. How do I work around that — JSP, Apr 06 '20 at 19:33

score 0 · Answer 1 · answered Apr 07 '20 at 13:51

Here is an example that is clearly NOT a solution for you, but an example of good practices that can answer some of your questions. You must modify it to suit your situation.

Try to avoid "magic numbers". Create a constant using a name that describes why the number exists.
Try and keep your top level logic "readable". If you can create a stand-alone block of code that only occurs once, or occurs repeatedly, it can more clearly show the "flow" of your logic.
Establish "working" variables that use names closely tied to either the data or the type of operation you're working with. In this case, I'm showing three worksheet objects. You should pick names that are meaningful to you and your application.
Always fully qualify your worksheet references.

Here's the code:

Option Explicit

Private Const NBcol As Long = 13
Private Const MZcol As Long = 16
Private Const BLcol As Long = 12

Sub CompareMyDataExample()

    ClearAllConditionalFormats

    Dim listWS As Worksheet
    Dim dataWS As Worksheet
    Dim infoWS As Worksheet
    Set listWS = ThisWorkbook.Sheet1
    Set dataWS = ThisWorkbook.Sheets("My Data")
    Set infoWS = ThisWorkbook.Sheet2

    Const START_ROW As Long = 7
    Const END_ROW As Long = 26

    Dim myList As Range
    Dim lastRow As Long
    With listWS
        lastRow = .Cells(.Rows.Count, 1).End(xlUp).Row
        Set myList = .Range("A1").Resize(lastRow, 1)
    End With

    Dim i As Long
    For i = START_ROW To END_ROW
        Dim nb As Long
        nb = infoWS.Cells(i, NBcol).Value
        If Not IsError(Application.Match(nb, myList, 0)) Then
            If infoWS.Cells(i, MZcol).Value >= 3.5 Then
                '--- always include ALL worksheet references with every
                '    reference to a part of a worksheet
                infoWS.Range(infoWS.Cells(i, BLcol), infoWS.Cells(i, MZcol)).Interior.Color = RGB(250, 191, 143)
            Else
                '--- as an alternative, you can set up a "with" block
                '    but ALWAYS remember to use the "." to make sure it
                '    links back to the "with" block reference
                With infoWS
                    .Range(.Cells(i, BLcol), .Cells(i, MZcol)).Interior.Color = RGB(197, 217, 241)
                End With
            End If
        End If
    Next i

End Sub

Private Sub ClearAllConditionalFormats()
    Dim ws As Worksheet
    For Each ws In ThisWorkbook.Worksheets
        ws.Cells.FormatConditions.Delete
    Next ws
End Sub

So I used those as variables since I broke that out into a sub trying to keep the main sub cleaner. Those column numbers will change as I go from table to table in each sheet. I created another sub for comparing to a - modified z score. I fixed all those references. I knew there was a reference structure, but was never really clear on which command types require which kind of structure. This helped me fix that in a lot of places. I saw the range dimension for mylist as I was working on figuring out how to do that (Thanks). — JSP, Apr 09 '20 at 14:40
To check all my tables across those 3 sheets I did an if inside a for to set the ws for each sheet that gets passed to everything, which would finally work since I went back and properly qualified the references. I got this all working a few days ago, and then proceeded on to sorting and hiding rows in the other sheets. I've been expanding what this does bit by bit. Thanks again for the guidance. It's certainly helped me speed up what I'm developing. — JSP, Apr 09 '20 at 14:42

VBA Excel IsError within For loop issues

1 Answers1