1

I have a dataset that contains a bunch of information on customers. I want this information moved into another worksheet when Yes is entered into the Cell in row K, and the original row containing the information to be deleted from the first worksheet

I'm doing this with a button press. Works fine, and rows were being deleted, but of course when it deletes the row then the row numbers shift up by one and so the next row is skipped over (row 15 is deleted and then row 16 becomes row 15 etc.) so I figured a reverse loop is the way to go, but I can't for the life of me figure this out - I thought it would be simple! Here's the code I'm using:

Private Sub UpdateSheet2_Click()

Dim c As Range
Dim Source As Worksheet
Dim Target As Worksheet

    'Change worksheet designation as needed
    Set Source = ActiveWorkbook.Worksheets("Sheet1")
    Set Target = ActiveWorkbook.Worksheets("Sheet2")

    For Each c In Source.Range("K:K") 'Covers all rows          
        If c = "yes" Then
            Source.Range("A" & c.Row & ":B" & c.Row).Copy Target.Range("A" & Rows.Count).End(xlUp).Offset(1, 0)
            Source.Range("D" & c.Row & ":F" & c.Row).Copy Target.Range("C" & Rows.Count).End(xlUp).Offset(1, 0)
            Source.Range("H" & c.Row & ":J" & c.Row).Copy Target.Range("F" & Rows.Count).End(xlUp).Offset(1, 0)    
        End If

'        If c = "yes" Then
'            Source.Rows(c.Row).EntireRow.Delete
'        End If
    Next c

    For i = 500 To 1 Step -1
        If Source.Range("K" & i) = "yes" Then
            Source.Rows(i.Row).EntireRow.Delete
        End If  
    Next i 

End Sub

I'm just setting the row numbers between 1 and 500 for now, I will alter this when I have the basic functionality working. You can see where I commented out the original deletion method in the For Each loop. I'm currently getting the error "Object required" on the line Source.Rows(i.Row).EntireRow.Delete

chris neilsen
  • 52,446
  • 10
  • 84
  • 123
Jenn Thorpe
  • 33
  • 3
  • 1
    Try changing `Source.Rows(i.Row).EntireRow.Delete` to `Source.Range("A" & i).EntireRow.Delete` should work fine. – Damian Jun 06 '19 at 08:03
  • 1
    Needs to be `Source.Rows(i).EntireRow.Delete`, the value in the brackets merely represents the row number which you have already defined with `i` – Tim Stack Jun 06 '19 at 08:04
  • 1
    Additionally, why loop through all 1,048,576 rows (and maybe more in future versions) in your `For Each` loop? Define the last filled row in col A with `Source.Cells(Source.Rows.Count, "A").End(xlUp).Row` – Tim Stack Jun 06 '19 at 08:07
  • Like @TimStack said, is how to do it on your example. The error comes when you want to reference de row number, `i` is already a row number, `i.Row` can't be, because `Row` is a `Range` property, and `i` is a numeric variable. Thought another tip is to use `Option Explicit` on the top of your module to force the decalration of all your variables. – Damian Jun 06 '19 at 08:07
  • 1
    Yes! Brilliant, thank you all :) makes totally sense now you point it out - duh! Tim - Thanks for the tips for my loop, I was planning to tidy it all up and make it efficient once I got the core stuff working :) will definitely be using your suggestion <3 – Jenn Thorpe Jun 06 '19 at 08:11
  • You can also "copy" values this way: `Target.Range(your range) = Source.Range(your range).Value` . – FAB Jun 06 '19 at 08:33
  • @JennThorpe have in mind that the copy paste method used copy both values & formatting which may cause delay in the whole process especially if you have many rows. – Error 1004 Jun 06 '19 at 08:41
  • @JennThorpe Performance on deleting rows can get downright awful. If your program grows and this becomes a bottleneck, have a look at this --> https://stackoverflow.com/questions/30959315/excel-vba-performance-1-million-rows-delete-rows-containing-a-value-in-less – Ryan Wildry Jun 06 '19 at 11:15

1 Answers1

5

It is generally more efficient to delete all your rows at once.

Set c = Nothing 'reset your range
For i = 1 To 500 'or 500 To 1 Step -1 - doesn't make a difference
    If Source.Range("K" & i) = "yes" Then
        If c Is Nothing Then
            Set c = .Cells(i, 1).EntireRow 'if the range is empty then set it to the required row
        Else
            Set c = Union(c, .Cells(i, 1)).EntireRow 'otherwise add this row
        End If
    End If
Next
If Not c Is Nothing Then c.Delete xlUp 'if the range is not empty then delete all the rows

(I should add that my use of Cells(x,y).EntireRow is purely a personal preference. I always use that format as I find it easier to debug.)