7

I'm trying to compare cell values between 2 Sheets (Sheet1 & Sheet2) to see if they match, and if they match move the matching values in Sheet1 to a pre-existing list (Sheet3) and delete the values in Sheet1 afterwards.

I'm using the reverse For Loop in Excel VBA, but everything works until the part where I start deleting the row using newrange1.EntireRow.Delete.

This throws a '424' Object Required Error in VBA and I've spent hours trying to solve this, I'm not sure why this is appearing. Am I selecting the row incorrectly? The object?

Would appreciate if anyone can point me to the correct direction.

Here's my code:

Sub Step2()

    Sheets("Sheet1").Activate

    Dim counter As Long, unsubListCount As Long, z As Long, x As Long, startRow As Long
    counter = 0
    startRow = 2
    z = 0
    x = 0

    ' Count Sheet3 Entries
    unsubListCount = Worksheets("Sheet3").UsedRange.Rows.Count

    Dim rng1 As Range, rng2 As Range, cell1 As Range, cell2 As Range, newrange1 As Range

    ' Select all emails in Sheet1 and Sheet2 (exclude first row)
    Set rng1 = Worksheets("Sheet1").Range("D1:D" & Worksheets("Sheet1").UsedRange.Rows.Count)
    Set rng2 = Worksheets("Sheet2").Range("D1:D" & Worksheets("Sheet2").UsedRange.Rows.Count)

   ' Brute Loop through each Sheet1 row to check with Sheet2
    For z = rng1.Count To startRow Step -1
        'Cells(z, 4)
        Set cell1 = Worksheets("Sheet1").Cells(z, "D")
        For x = rng2.Count To startRow Step -1
            Set cell2 = Worksheets("Sheet2").Cells(x, "D")

            If cell1.Value = cell2.Value Then ' If rng1 and rng2 emails match
                counter = counter + 1
                Set newrange1 = Worksheets("Sheet1").Rows(cell1.Row)

                newrange1.Copy Destination:=Worksheets("Sheet3").Range("A" & unsubListCount + counter)
                newrange1.EntireRow.Delete
            End If
        Next
    Next
End Sub

Here's the error I'm getting:

424 Object Required

Jordan
  • 4,424
  • 2
  • 18
  • 32
Kyle Yeo
  • 2,270
  • 5
  • 31
  • 53
  • Looks fine to me. What does `newrange1` contain in the locals window? – GSerg Mar 07 '17 at 14:25
  • its b/c newrange1 is assigned to Worksheets("Sheet1").Rows(cell1.Row). It might be easy to get active cell's row? a = ActiveCell.Row – Doug Coats Mar 07 '17 at 14:25
  • @GSerg what's a locals window? I'm quite new to VBA, i must add... 1 day old, :P – Kyle Yeo Mar 07 '17 at 14:26
  • I tried to reproduce your problem with the following short sub: `Dim newrange1 As Range, cell1 As Range : Set cell1 = Worksheets("Sheet2").Range("a1") : Set newrange1 = Worksheets("Sheet1").Rows(cell1.Row) : newrange1.EntireRow.Delete` But that works fine (no errors). Do you have the sheet protected? – Ralph Mar 07 '17 at 14:27
  • @KyleYeo https://msdn.microsoft.com/en-us/library/office/gg264148.aspx – GSerg Mar 07 '17 at 14:28
  • Is that code the actual code you are running? It feels like your actual code has either more or less parentheses. – GSerg Mar 07 '17 at 14:31
  • 4
    a) Your inner loop produces a lot of unnecessary work. Better with `Application.Match`. –  Mar 07 '17 at 14:32
  • @DougCoats that's the thing I want to avoid the macro running on the active sheet, so i'm redirecting it to the correct sheet -- am I not supposed to do it this way? – Kyle Yeo Mar 07 '17 at 14:32
  • @GSerg yes it is -- the ones I commented out are Msgbox statements for debugging. – Kyle Yeo Mar 07 '17 at 14:33
  • @Ralph no it isn't protected -- just double-checked. – Kyle Yeo Mar 07 '17 at 14:35
  • @DougCoats Please see http://stackoverflow.com/q/10714251/11683. The OP is correct in using object references as opposed to `ActiveCell`. – GSerg Mar 07 '17 at 14:36
  • Yes it def more preferred to reference what the current state of the loop is and then find the current row off of that. – Doug Coats Mar 07 '17 at 14:40
  • @KyleYeo Are you sure that is [in fact](http://stackoverflow.com/a/4113134/11683) the line where the error occurs? – GSerg Mar 07 '17 at 14:40
  • 1
    Just an idea because I once had a somewhat related problem: insert new lines of code such as `Set cell1 = Nothing` and `Set newrange1 = Nothing` **before** re-assigning them to new ranges (maybe at the end of the loops just before the ...next...). – Ralph Mar 07 '17 at 14:41
  • @GSerg ok using your post I set it to break on all errors -- now I'm getting this as the highlighted sentence when I click on Debug: `If cell1.Value = cell2.Value Then ' If rng1 and rng2 emails match` -- does this mean this is the issue? – Kyle Yeo Mar 07 '17 at 14:44
  • Could be to do with `Set rng1 = Worksheets("Sheet1 ")` <- There's an additional space in this sheet name. – Jordan Mar 07 '17 at 14:45
  • @Jordan I rectified that -- the original one had no spaces there. – Kyle Yeo Mar 07 '17 at 14:46
  • @GSerg the weird thing is, if I don't add the `.EntireRow.Delete` functionality, the entire script runs without any issues, so I thought this would be the function. I ran the debug again with the tools you provided but without the `.EntireRow.Delete` section again it goes smooth and doesn't throw anything up. – Kyle Yeo Mar 07 '17 at 14:48
  • Do you have events handlers, especially for `Worksheet_Changed`? – GSerg Mar 07 '17 at 14:49
  • @GSerg Nope, apart from this `Sub` I only have 2 other functions -- 1 to delete empty rows, 1 to check against a cell value in Sheet1 and clear that cell value if it matches. – Kyle Yeo Mar 07 '17 at 14:52
  • Try to add `Exit for` before `End If`. – Fadi Mar 07 '17 at 14:52
  • @KyleYeo Then we're back to [the locals window](http://stackoverflow.com/questions/42650672/excel-vba-runtime-error-424-object-required-when-deleting-rows?noredirect=1#comment72427637_42650672). What does it contain before executing `.EntireRow.Delete`? – GSerg Mar 07 '17 at 14:54
  • @GSerg yeah I opened the locals window up -- it contains a `Range/Range` Type before `.EntireRow.Delete` is executed. There are a lot of values in there though. – Kyle Yeo Mar 07 '17 at 14:56
  • @KyleYeo Then add `cell1`, `cell2` and `newrange1` to the watch window instead ant watch only them. Also execute the code step by step by pressing F8. – GSerg Mar 07 '17 at 15:00
  • 1
    ... and check `? newrange1.Address` in the immediate window just before you execute `.EntireRow.Delete`. https://www.excelcampus.com/vba/vba-immediate-window-excel/ – Ralph Mar 07 '17 at 15:02
  • @GSerg from what I see in debug I am able to delete 1 entry that matches before getting the '424' Object Required error. I'll look into the `cell1`, `cell2`, `newrange1` and `newrange1.Address` -- this watch window should help loads. – Kyle Yeo Mar 07 '17 at 15:03
  • @GSerg @Ralph doing `Msgbox newrange1.Address` gives me "$13:$13". That's a valid address, right? – Kyle Yeo Mar 07 '17 at 15:11
  • 1
    @GSerg @Ralph it seems @Ryan might have solved my issue -- I think that's pretty much it -- I need to set `newrange1` to `Nothing`, then reset `cell1` to the next cell. Thanks guys! – Kyle Yeo Mar 07 '17 at 15:17

2 Answers2

5

Your inner loop produces a lot of step-by-step work that is better accomplished with Application.Match. Your use of .UsedRange to retrieve the extents of the values in the D columns is better by looking for the last value from the bottom up.

Option Explicit

Sub Step2()

    Dim z As Long, startRow As Long
    Dim rng2 As Range, wk3 As Worksheet, chk As Variant

    startRow = 2
    z = 0
    Set wk3 = Worksheets("Sheet3")

    ' Select all emails in Sheet1 and Sheet2 (exclude first row)
    With Worksheets("Sheet2")
        Set rng2 = .Range(.Cells(2, "D"), .Cells(.Rows.Count, "D").End(xlUp))
    End With

    With Worksheets("Sheet1")
        For z = .Cells(.Rows.Count, "D").End(xlUp).Row To startRow Step -1
            chk = Application.Match(.Cells(z, "D").Value2, rng2, 0)
            If Not IsError(chk) Then
                .Cells(z, "A").EntireRow.Copy _
                    Destination:=wk3.Cells(Rows.Count, "A").End(xlUp).Offset(1, 0)
                .Cells(z, "A").EntireRow.Delete
            End If
        Next
    End With
End Sub

As noted by Ryan Wildry, your original problem was continuing the loop and comparing after deleting the row. This can be avoided by adding Exit For after newrange1.EntireRow.Delete to jump out of the inner loop once a match was found. I don't think you should 'reset cell1' as this may foul up the loop iteration.

Community
  • 1
  • 1
3

I think what's happening is when you are deleting the row, you are losing the reference to the range Cell1. So I reset this after the deletion is done, and removed the reference to newRange1. Give this a shot, I have it working on my end. I also formatted the code slightly too.

Option Explicit

Sub Testing()

    Dim counter         As Long: counter = 0
    Dim z               As Long: z = 0
    Dim x               As Long: x = 0
    Dim startRow        As Long: startRow = 2
    Dim Sheet1          As Worksheet: Set Sheet1 = ThisWorkbook.Sheets("Sheet1")
    Dim Sheet2          As Worksheet: Set Sheet2 = ThisWorkbook.Sheets("Sheet2")
    Dim Sheet3          As Worksheet: Set Sheet3 = ThisWorkbook.Sheets("Sheet3")
    Dim rng1            As Range: Set rng1 = Sheet1.Range("D1:D" & Sheet1.UsedRange.Rows.Count)
    Dim rng2            As Range: Set rng2 = Sheet2.Range("D1:D" & Sheet2.UsedRange.Rows.Count)
    Dim unsubListCount  As Long: unsubListCount = Sheet3.UsedRange.Rows.Count
    Dim cell1           As Range
    Dim cell2           As Range
    Dim newrange1       As Range

   ' Brute Loop through each Sheet1 row to check with Sheet2
    For z = rng1.Count To startRow Step -1
        Set cell1 = Sheet1.Cells(z, 4)
        For x = rng2.Count To startRow Step -1
            Set cell2 = Sheet2.Cells(x, 4)
            If cell1 = cell2 Then
                counter = counter + 1
                Set newrange1 = Sheet1.Rows(cell1.Row)
                newrange1.Copy Destination:=Sheet3.Range("A" & unsubListCount + counter)
                newrange1.EntireRow.Delete
                Set newrange1 = Nothing
                Set cell1 = Sheet1.Cells(z, 4)
            End If
        Next
    Next
End Sub
Ryan Wildry
  • 5,612
  • 1
  • 15
  • 35
  • To note, basically adding these 2 lines `Set newrange1 = Nothing` and `Set cell1 = Sheet1.Cells(z, 4)` after `newrange1.EntireRow.Delete` solved it. Thanks! – Kyle Yeo Mar 07 '17 at 15:19
  • 1
    @KyleYeo I guess you missed my comments in that respect a while back: http://stackoverflow.com/questions/42650672/excel-vba-runtime-error-424-object-required-when-deleting-rows#comment72428204_42650672 Then we could have solved it a lot sooner. RyanWildry: Thanks for making a fully-fledged reply out of that. – Ralph Mar 07 '17 at 15:28
  • @Ralph yeah I realized -- too many comments coming in at the same time... – Kyle Yeo Mar 07 '17 at 15:34