Macro to copy data runs too long?

[spyldbrat]

Hello,

I have a spreadsheet with 6300 lines. The number of lines in this spreadsheet can vary from week to week.

I have the following macro which looks at column s, looks for "ABC Company", copies the entire row and pastes it on "Tab Client X". It's should find the first empty row starting with row 2.

when I try to run this macro, it ran for SEVERAL minutes before I finally stopped it. I am not sure if it's taking so long because the number of rows in the spreadsheet or because I have something wrong? I know that one issue I have is where it pastes, i don't have it "set" to paste it to the first empty row.

Set MR = Range("S:S")

For Each cell In MR
Sheets("Client").Select
If cell.Value = "ABC Company" Then
cell.EntireRow.Copy
Sheets("Tab Client X").Select
Range("a2").Select
ActiveSheet.Paste
End If
Next cell
End Sub

 

[Johnny Thunder]

You need to remove the "SELECT" lines of code,

Something like this,

Untested:

Set MR = Range("S:S")


With Sheets("Clients")


For Each cell In MR
If cell.Value = "ABC Company" Then
cell.EntireRow.Copy
End with 


With SHeets("Tab Client X")
Range("a2").Paste


End If


Next cell


End Sub

 

[Joe4]

The big problem is that you are looping through ALL the rows in column S, regardless of where you data may end! Since that could be over 1,000,000 lines long, that is lot of unnecessary processing.
If you had wanted to go the way of loops (which are pretty inefficient, you could do it like this):

Sub MyCopy()

    Dim lr As Long
    Dim MR As Range
    Dim rw As Long
    Dim cell As Range
    
    Application.ScreenUpdating = False
    
'   Set initial value of rw
    rw = 2
    
'   Find last row with data in column S
    lr = Sheets("Client").Cells(Rows.Count, "S").End(xlUp).Row
    
    Set MR = Sheets("Client").Range("S1:S" & lr)

    For Each cell In MR
        If cell.Value = "ABC Company" Then
            cell.EntireRow.Copy
            Sheets("Tab Client X").Select
            Cells(rw, "A").Select
            ActiveSheet.Paste
            rw = rw + 1
        End If
    Next cell
    
    Application.ScreenUpdating = True
    
End Sub

However, a much more efficient way would be to used Advanced Filters, and filter to a new location.
See here: https://www.extendoffice.com/documents/excel/4189-excel-dynamic-filter-to-new-sheet.html

 

[Fluff]

By setting MR to Col S you're checking all 1,048,576 cells, hence the time taken.
Do you just want to look for "ABC Company", or will you want to look for various values?

 

[spyldbrat]

I got a compile error message at "END WITH". It says "End With without With.

For Each cell In MR
If cell.Value = "ABC Company" Then
cell.EntireRow.Copy
End with

 

[Joe4]

Have you tried my suggestions?

 

[spyldbrat]

By setting MR to Col S you're checking all 1,048,576 cells, hence the time taken.
Do you just want to look for "ABC Company", or will you want to look for various values?

I just want to look for ABC Company.

 

[spyldbrat]

Have you tried my suggestions?

Joe4: I did not try your suggestions because you had said it was not the most efficient way of doing what I need to. I figured you modified my macro so it would run but just not the most efficient way? Did I misunderstand what you were trying to tell me?

 

[Joe4]

Joe4: I did not try your suggestions because you had said it was not the most efficient way of doing what I need to. I figured you modified my macro so it would run but just not the most efficient way? Did I misunderstand what you were trying to tell me?

Loops are not the most efficient thing to use when there are viable alternatives. However, there are ways of taking a bad loop and making it more efficient. I just wanted to show you how you could take your original process and speed it up. The three keys there are to:
- limit it to only run on the number of rows of data you have, and not any more
- suppress Screen Updating while it is running
- limiting the number of Select statements you use

If you run the code I provided, I think you will find it should be MUCH faster than what you had previously (try it and see).
By the way, your original code and the the other suggestion also use loops.

However, it can be made even more efficient and faster to use other methods that don't involve loops, like Advanced Filters which allows you to filter your data on a value and write the results to another sheet. This sounds EXACTLY what you want to do. Did you take a look at the link that I provided that shows you how to do that?

By the way, your original code and the the other suggestion also use loops.

 

[Fluff]

Untested, but try

Sub chk()
   With Sheets("Clients")
      .Range("S:S").AutoFilter 1, "ABC Company"
      .UsedRange.Offset(1).SpecialCells(xlVisible).Copy Sheets("Tab Client X").Range("A2")
      .AutoFilterMode = False
   End With
End Sub