VBA Union Not Working As Desired

Question

My code is not working. It breaks on the uni.entirerow.delete line, with error 1004. That indicates that it is building the array, but I must be referencing something wrong? I originally was just having it delete line=by=line, but it takes too long, and there are about 600k rows to go through.

The macro is supposed to reference if the value of Column B equals any of the values of p from ws2. If it does, add it to uni and then delete after the range is fully reviewed. Union is supposed to work much faster than deleting line by line.

Any ideas? Full code below:

Option Explicit
Sub TrimOut()
Dim wb As Workbook
Dim ws1 As Worksheet
Dim ws2 As Worksheet
Dim i As Long, p As Long
Dim uni As Range

Application.Calculation = xlCalculationManual


Set wb = ThisWorkbook
Set ws1 = wb.Sheets(1)
Set ws2 = wb.Sheets(2)

For i = 610197 To 591043 Step -1
    For p = 8 To 82
        If ws1.Range("B" & i).Value = ws2.Range("A" & p).Value Then
            'ws1.Rows(i).Delete old snippet, works fine
            If uni Is Nothing Then
                Set uni = ws1.Cells(i, 1).EntireRow
            Else
                Set uni = Application.Union(uni, ws1.Cells(i, 1).EntireRow)
            End If
        End If
    Next p
Next i

If Not uni Is Nothing Then
uni.EntireRow.Delete
End If

Application.Calculation = xlCalculationAutomatic
End Sub

EDIT: Here is a 'working' code, though not optimal.

Option Explicit
Sub TrimOut()
Dim wb As Workbook
Dim ws1 As Worksheet
Dim ws2 As Worksheet
Dim i As Long, p As Long
Dim uni As Range
Dim count As Long

Application.Calculation = xlCalculationManual


Set wb = ThisWorkbook
Set ws1 = wb.Sheets(1)
Set ws2 = wb.Sheets(2)

count = 0

For i = 607257 To 1 Step -1
    For p = 8 To 82
        If ws1.Range("B" & i).Value = ws2.Range("A" & p).Value Then
            'ws1.Rows(i).Delete
            If uni Is Nothing Then
                Set uni = ws1.Rows(i)
            Else
                Set uni = Excel.Union(uni, ws1.Rows(i))
                count = count + 1
            End If
        End If

        If count > 1000 Then
            uni.Delete
            count = 0
        End If
    Next p
Next i

If Not uni Is Nothing Then
    uni.Delete
End If

Application.Calculation = xlCalculationAutomatic
End Sub

Answer 1

I've been running the below code successfully. I need to look at some metrics excluding 'top 5', 'top 10', etc for p = 1 to 958 for some different reports. Thank you all for your help.

It appears the "real" issue was that excel doesn't want to delete groups of rows that aren't grouped, and throws an absolute hissy-fit.

It might be optimal to move the count loop before p, but whatever.

Option Explicit
Sub TrimOut()
Dim wb As Workbook
Dim ws1 As Worksheet
Dim ws2 As Worksheet
Dim i As Long, p As Long
Dim uni As Range
Dim count As Long
Dim lrow As Long


Application.Calculation = xlCalculationManual


Set wb = ThisWorkbook
Set ws1 = wb.Sheets(1)
Set ws2 = wb.Sheets(2)

lrow = ws1.Range("A" & ws1.Rows.count).End(xlUp).Row

For i = lrow To 1 Step -1
    For p = 1 To 82
        If Not uni Is Nothing Then
            If count > i + 1 Then
                uni.Delete
                Set uni = Nothing
            End If
        End If

        If ws1.Range("B" & i).Value = ws2.Range("A" & p).Value Then
            If uni Is Nothing Then
                Set uni = ws1.Rows(i)
            Else
                Set uni = Excel.Union(uni, ws1.Rows(i))
            End If
            count = i
        End If
    Next p
Next i

If Not uni Is Nothing Then
    uni.Delete
End If

Application.Calculation = xlCalculationAutomatic
End Sub

Answer 2

I have just run your code and it seems that the slowness is not in the deletion, but in the nested loops. You iterate more than 1.4 million times ((610197-591043)*(82-8)) and each time you are probably changing a range. That is usually slow. Think of another way to do it, e.g. record the data in array or list and then try further.

To see yourself, write Stop on the line after Next i and see how quick it reaches it. Then the deletion is quite fast.