IF statement being ignored by excel

Question

I am having a problem with a small part of a macro. Essentially, what it is supposed to do is check if a value is found whithin a column, if it isn't then it continues through the rest of the code:

Dim p As Long
Dim j As Long
Dim LastRow2 As Long
Dim LastRow3 As Long
Dim FindRow As Range
Dim a As Worksheet
Dim b As Worksheet
Dim c As Worksheet
Dim d As Worksheet

Set a = Workbooks("Item ID Comparison").Worksheets(1)
Set b = Workbooks("Item ID Comparison").Worksheets(2)
Set c = Workbooks("Item ID Comparison").Worksheets(3)
Set d = Workbooks("Prueba").Worksheets(1)
LastRow3 = d.Cells(Rows.Count, "F").End(xlUp).Row

For p = 2 to LastRow3

    If b.Columns(6).Find(d.Cells(p, 6), lookat:=xlWhole) Is Nothing Then
        Set FindRow = d.Columns(6).Find(a.Cells(p, 7))
        j = FindRow.Row
        If Not a.Columns(7).Find(d.Cells(p, 6), lookat:=xlWhole) Is Nothing Then
            LastRow2 = c.Cells(Rows.Count, "A").End(xlUp).Row
            c.Range("A" & LastRow2 + 1).Value = d.Cells(j, "A").Value
            c.Range("B" & LastRow2 + 1).Value = d.Cells(j, "B").Value
            c.Range("C" & LastRow2 + 1).Value = d.Cells(j, "C").Value
            c.Range("D" & LastRow2 + 1).Value = d.Cells(j, "D").Value
            c.Range("E" & LastRow2 + 1).Value = d.Cells(j, "E").Value
            c.Range("F" & LastRow2 + 1).Value = d.Cells(j, "F").Value
            c.Range("G" & LastRow2 + 1).Value = d.Cells(j, "G").Value
            c.Range("H" & LastRow2 + 1).Value = d.Cells(j, "H").Value
            c.Range("I" & LastRow2 + 1).Value = d.Cells(j, "I").Value
        End If
    End If
Next p

The problem is that excel seems to be ignoring the IF statement for some reason (while actually following the second IF statement. I end up finding values within the data-set that were found within b.columns(6).

Any help would be greatly appreciated.

Answer 1

Ignore my other answer. I missed that you were qualifying your two ranges with two different worksheets. Lets try this again, this time though we will use our debugging tools to see if we can figure out what is going on.

First, i'll provide some edited code, and then we can walk through it and see what the changes I've made are, as well as look at how debugging can help figure out what is happening:

Sub Foo()
    ' Descriptive names make your variables easy to follow.
    ' It is also best to declare your variables AS CLOSE to their first use as possible.
    ' Since below this point we start using Prueba, and then we get into the loop, this is the ideal location.
    With Application.Workbooks("Item ID Comparison")
        Dim FirstComparison As Worksheet
        Set FirstComparison = .Worksheets(1)

        Dim SecondComparison As Worksheet
        Set SecondComparison = .Worksheets(2)

        Dim ThirdComparison As Worksheet
        Set ThirdComparison = .Worksheets(3)
    End With

    Dim Prueba As Worksheet
    Set Prueba = Application.Workbooks("Prueba").Worksheets(1)

    ' Note here that, previously, 'Rows' was not properly qualified to 'Prueba' and thus actually refers to 'ActiveSheet.Rows.Count'
    'LastRow3 = d.Cells(Rows.Count, "F").End(xlUp).Row

    Dim LastPruebaRow As Long
    Prueba.Cells(Prueba.Rows.Count, "F").End(xlUp).Row

    ' By convention, we start with i and j as control variables
    Dim i As Long
    For i = 2 To LastPruebaRow
        Dim SearchResult As Range

        ' Note here as well that I specifically call on the value of the cell, not the cell itself.
        ' While the default member of a 'Cell' is 'Value' it is far better to be explicit than to be implicit.
        Set SearchResult = SecondComparison.Columns(6).Find(Prueba.Cells(i, 6).Value, lookat:=xlWhole)

        ' By separating out the concerns, you can look at the result of the find if it is successful, or use the immediate window
        ' to test different values of 'Find' until 'Not SearchResult Is Nothing'
        If SearchResult Is Nothing Then
            Dim FindRow As Range

            ' Again, specifically call on the 'Value' member
            Set FindRow = Prueba.Columns(6).Find(FirstComparison.Cells(i, 7).Value)

            Dim ResultRow As Long
            ResultRow = FindRow.Row

            ' And again, get 'Value' from the cell.
            If Not FirstComparison.Columns(7).Find(Prueba.Cells(i, 6).Value, lookat:=xlWhole) Is Nothing Then
                ' Note again that the row range is unqualified here, and actually is referring to 'ActiveSheet.Rows.Count'
                ' c.Cells(Rows.Count, "A").End(xlUp).Row

                With ThirdComparison
                    ' Since you are only ever using the index of the row + 1, just merge the two.
                    Dim LastComparisonRow As Long
                    LastComparisonRow = .Cells(.Rows.Count, "A").End(xlUp).Row + 1

                    .Range("A" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "A").Value
                    .Range("B" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "B").Value
                    .Range("C" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "C").Value
                    .Range("D" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "D").Value
                    .Range("E" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "E").Value
                    .Range("F" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "F").Value
                    .Range("G" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "G").Value
                    .Range("H" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "H").Value
                    .Range("I" & LastComparisonRow).Value = Prueba.Cells(ResultRow, "I").Value
                End With
            End If
        End If
    Next i
End Sub

The first thing I think is worth pointing out is that it is much easier to read what your code is doing (at least for others) when everything has a descriptive name. It didnt take me long to do this, or to come up with names that made sense, but it made it much easier to follow the flow of your code.

Likewise, all variable declaration is now happening as close to the first use as possible. I dont need to look at your Dim block at the top to know that LastPruebaRow is a long, it's right there (and its much easier to know what LastPruebaRow is over LastRow3).

Second, when using numeric variables for for loops the convention is to start with i. Thus:

For i = 1 to 10
    For j = 2 to 20
        For k = 3 to 30

        Next
    Next
Next

So on, so forth, though if you find yourself past j (and more often than not, past i) you likely need to refactor your code.

After adjusting the names and declarations, things started to pop out. For example:

LastRow3 = d.Cells(Rows.Count, "F").End(xlUp).Row

Isn't actually returning the last row of the Prueba worksheet. Instead, it is returning:

ThirdComparisonSheet.Cells(ActiveSheet.Rows.Count, "F").End(xlUp).Row

You also have this bit of code that doesn't conform to the rest:

Set FindRow = Prueba.Columns(6).Find(FirstComparison.Cells(i, 7).Value)

Notice how the others have lookat:=xlWhole and this one doesn't? This may be by design, so I left it, but it stuck out to me.

I also find it a little bit odd that your first conditional is looking for Nothing:

    ' Note here as well that I specifically call on the value of the cell, not the cell itself.
    ' While the default member of a 'Cell' is 'Value' it is far better to be explicit than to be implicit.
    Dim SearchValue as Variant
    SearchValue = Prueba.Cells(i, 6).Value

    Debug.Print "i :", i, "SearchValue :", SearchValue
    Dim SearchResult As Range
    Set SearchResult = SecondComparison.Columns(6).Find(SearchValue, lookat:=xlWhole)
    If SearchResult Is Nothing Then

Lastly, as you can also see in the above example, the loop will print out i and SearchValue on every iteration (use this with caution, I highly recommend using a breakpoint, particularly with large worksheets).

In all of this, we should have a much easier time finding the problem (if it hasnt been surfaced already).

Answer 2

I think this may be the source of your issue:

For p = 2 to LastRow3

    ' Note that we are looking in Column(6) for a value we are finding in 
    ' Cell(p, 6) which means that Column(6) will always contain the value
    ' (since we just took the value from column 6).
    If b.Columns(6).Find(d.Cells(p, 6), lookat:=xlWhole) Is Nothing Then
        Set FindRow = d.Columns(6).Find(a.Cells(p, 7))
        j = FindRow.Row
        If Not a.Columns(7).Find(d.Cells(p, 6), lookat:=xlWhole) Is Nothing Then

        End If
    End If
Next p

In essence, you have are using Find on Columns(6) with a value from Cells(p, 6). Since you are looking for a value in the same column that you are getting your value from, this condition will always be True except in the very rare instances where the value is removed between passing the value to Find and actually looking for it (very very unlikely scenario).