2

I'm finishing a script that verifies if a cell in Column A of Sheet1 ("INCIDENTS") is duplicated at Column A of Sheet2 ("INCDB") and if the cell is duplicate it deletes the whole row in Sheet1.

The problem is that after the first loop (and deleting the row) it gives me the 424 error and highlights If iSrc.Cells.Value = iDst.Cells.Value Then

Any ideas on the cause? Here's the code:

Sub CTDeleteDuplicatePaste()

Dim ws1 As Worksheet
Dim ws2 As Worksheet
Dim iSrc As Variant
Dim iDst As Variant
Dim rng As Range

Set ws1 = Sheets("INCIDENTS")
Set ws2 = Sheets("INCDB")

For Each iSrc In ws1.Range("A5:A9999" & LastRow)
    For Each iDst In ws2.Range("A5:A9999")
        If iSrc.Cells.Value = iDst.Cells.Value Then
        If rng Is Nothing Then
            Set rng = iSrc.EntireRow
        Else
            Set rng = Union(rng, iSrc.EntireRow)
        End If
            rng.EntireRow.Delete

        End If

    Next iDst
Next iSrc

End Sub
ZygD
  • 22,092
  • 39
  • 79
  • 102
  • Try defining your range in reverse, e.g. `A9999:A5`. I'm curious if that would fix the issue since the behavior of Excel is pull rows up once you delete them. – Marc Oct 26 '15 at 20:48
  • Have you tried removing the `.cells`? – findwindow Oct 26 '15 at 20:52
  • 1
    I would also replace the `if rng ... End If rng.EntireRow.Delete`with just `iSrc.EntireRow.Delete` because there is no sense in taking the `Union` of a range that you have already deleted. You then don't need the `rng` variable either. – trincot Oct 26 '15 at 20:59
  • @Marc didn't work at all :( – Ugo Portela Pereira Oct 26 '15 at 20:59
  • @findwindow removing the .cells didn't change anything. It was able to delete one line and then it stopped. The debugger marked the same line. – Ugo Portela Pereira Oct 26 '15 at 21:01
  • @trincot thanks for the tip. Although it didn't change the outcome (as expected, I believe). I, obviously, took this code from somewhere else and I'm not quite sure about why the Union command was there or what it does. – Ugo Portela Pereira Oct 26 '15 at 21:03

1 Answers1

1

I'd do it without objects iSrc and iDst. And from reverse order - this code worked for me:

Sub CTDeleteDuplicatePaste()

Dim ws1 As Worksheet
Dim ws2 As Worksheet
Dim i As Long
Dim j As Long

Set ws1 = Sheets("INCIDENTS")
Set ws2 = Sheets("INCDB")

For i = 9 To 5 Step -1        'change 9 to 9999 for your real data
    For j = 9 To 5 Step -1    'change 9 to 9999 for your real data
        If Len(ws1.Cells(i, 1).Value) > 0 Then
            If ws1.Cells(i, 1).Value = ws2.Cells(j, 1).Value Then
                ws1.Cells(i, 1).EntireRow.Delete
                GoTo nextIteration
            End If
        End If
    Next
nextIteration:
Next

End Sub

Regarding the performance issue of .EntireRow.Delete, this is the additional reading:
Tests on processing 1 million rows
Solution employing Sorting

Community
  • 1
  • 1
ZygD
  • 22,092
  • 39
  • 79
  • 102
  • I'm going home and I will try that when I'm there. Do I have to Dim i and j as Variables? I'm trying to understand the properties concept in VBScript and programming in general. – Ugo Portela Pereira Oct 26 '15 at 21:07
  • Yes, you need to dim them - `As Long`. Also, I edited your main question - you don't use VBScript here. It's something similar, but it's called VBA. – ZygD Oct 26 '15 at 21:09
  • I think something went wrong. The code has been running for quite a while (10 minutes!!) and it only says that it's still running. I've pasted the code between `set ws2` and `End Sub`. And before that I set i and j `As Long`. I had to shut Excel from Task Manager. – Ugo Portela Pereira Oct 26 '15 at 21:48
  • it is something I had from another script. I ran this with i = 999 and j = 999 and it worked way better. One thing I found weird was that it deleted all the lines from 999 to 5 that didn't meet the value. Is that because "" = ""? If so, how could we add an exception? – Ugo Portela Pereira Oct 26 '15 at 22:04
  • Exactly. I have updated the code with one additional `If` to check if a cell in ws1 is not empty. Also, if using `LastRow` you can just replace `9999` with `LastRow` and that should be fine. – ZygD Oct 26 '15 at 22:11
  • it worked perfectly and now it doesn't even take a second to process. Thanks, man! – Ugo Portela Pereira Oct 26 '15 at 22:16
  • Alright, not that perfectly. I had forgotten to set the number to 999 (I currently have 600 rows with data) and it took ~5 seconds. It took over 5 minutes to process with 9999. @Zygd would the optimal way be to consider `i` and `j` the number of the last row in each spreadsheet? – Ugo Portela Pereira Oct 26 '15 at 22:25
  • Yes, `i` and `j` should begin with the number of the last row in respective sheet. However, that might not help much. `.EntireRow.Delete` is a drag on performance. There is not much we can do. One option could be just save the numbers of rows we need to delete in some kind of array. And only after we did it - delete rows in reverse order. But this way we would still have the same `.EntireRow.Delete`, not much of help out of that. – ZygD Oct 26 '15 at 22:32
  • Alright mate. I will try my way with counting the i and j's last row! Thank you very much. – Ugo Portela Pereira Oct 26 '15 at 22:43
  • If that's important to you, this is the additional reading on the performance problem of `.EntireRow.Delete`: [#1](http://stackoverflow.com/questions/30959315/excel-vba-performance-1-million-rows-delete-rows-containing-a-value-in-less) and [#2](http://stackoverflow.com/questions/14241882/entirerow-delete-performance-issue) – ZygD Oct 26 '15 at 22:46