Cleaning up some old code to remove SELECT, now works only in F8 debug mode

[logandiana]

Hi everyone, new member here.
I've never had any formal training on VBA, I've always just read a little here and there, and then gone the trial and error route.
I was working on adding some new procedures to a coworkers code and thought I'd try to modify his code a bit in an attempt to make it faster.
I've always heard that if it ain't broke don't fix it, but then I've also heard in this community to try to avoid using SELECT.

Here's a part of his original code. It sorts on one page, then copies to a specific sheet based on sales region. Then goes back and re-sorts and then copies to a different sheet. It does this same back and forth procedure for around 30 different sheets in the same workbook.

Sheets("Surgical Backorders").Select
    Range("A1").Select
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "3102", "3103", "3104", _
        "3201", "3202", "3203", "3204", "3205", "3301", "3302", "3303", "3304", "3305", "3401", _
        "3402", "3403", "3404", "3501", "3502", "3503", "3504", "3505", "3601", "3602", "3603", _
        "3604", "3605"), Operator:=xlFilterValues
    Rows("1:1").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.Copy
    Sheets("Northeast").Select
    Range("A1").Select
    Selection.PasteSpecial Paste:=xlPasteAllUsingSourceTheme, Operation:=xlNone _
        , SkipBlanks:=False, Transpose:=False
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Cells.Select
    Cells.EntireColumn.AutoFit
    Range("A1").Select
 
    Sheets("Surgical Backorders").Select
    Application.CutCopyMode = False
    Range("A1").Select
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "4101", "4102", "4103", "4104", "4201", "4202", "4203", "4204", "4301", "4302", "4303", _
        "4304", "4401", "4402", "4403", "4404", "4501", "4502", "4503", "4504"), Operator:= _
        xlFilterValues
    Rows("1:1").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.Copy
    Sheets("Southeast").Select
    Range("A1").Select
    Selection.PasteSpecial Paste:=xlPasteAllUsingSourceTheme, Operation:=xlNone _
        , SkipBlanks:=False, Transpose:=False
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Cells.Select
    Cells.EntireColumn.AutoFit
    Range("A1").Select
 
    Sheets("Surgical Backorders").Select

etc etc etc for around 30 sheets

Going with the 'SELECT' is bad mentality I changed the code to:

Sheets("Surgical Backorders").Select
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "3102", "3103", "3104", _
        "3201", "3202", "3203", "3204", "3205", "3301", "3302", "3303", "3304", "3305", "3401", _
        "3402", "3403", "3404", "3501", "3502", "3503", "3504", "3505", "3601", "3602", "3603", _
        "3604", "3605"), Operator:=xlFilterValues
    Range("A1:V1", Selection.End(xlDown)).Copy       
    Sheets("Northeast").Range("A1").PasteSpecial Paste:=xlPasteValues
    Sheets("Northeast").Range("A1").PasteSpecial Paste:=xlPasteFormats
    Sheets("Northeast").Cells.EntireColumn.AutoFit
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "4101", "4102", "4103", "4104", "4201", "4202", "4203", "4204", "4301", "4302", "4303", _
        "4304", "4401", "4402", "4403", "4404", "4501", "4502", "4503", "4504"), Operator:= _
        xlFilterValues
    Range("A1:V1", Selection.End(xlDown)).Copy
    Sheets("Southeast").Range("A1").PasteSpecial Paste:=xlPasteValues
    Sheets("Southeast").Range("A1").PasteSpecial Paste:=xlPasteFormats
    Sheets("Southeast").Cells.EntireColumn.AutoFit

etc etc etc for around 30 sheets.

This code works with no issues when I am F8 stepping through in debug mode.
When I just run the macro wide open from the beginning it really screams! It's so much faster, but the end result is wrong. It takes the same sorted values that should be pasted on the first sheet and it pasted them to every other sheet in the workbook.
It's like the macro is going too fast for the sorting part to catch up.

After spending half the day trying to figure this out. I finally added back in a few of the select and now the code works again.
Here's the code after I added back in some of the selects.

Sheets("Surgical Backorders").Select
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "3102", "3103", "3104", _
        "3201", "3202", "3203", "3204", "3205", "3301", "3302", "3303", "3304", "3305", "3401", _
        "3402", "3403", "3404", "3501", "3502", "3503", "3504", "3505", "3601", "3602", "3603", _
        "3604", "3605"), Operator:=xlFilterValues
    Range("A1:V1", Selection.End(xlDown)).Copy
    Sheets("Northeast").Select
    Sheets("Northeast").Range("A1").PasteSpecial Paste:=xlPasteValues
    Sheets("Northeast").Range("A1").PasteSpecial Paste:=xlPasteFormats
    Sheets("Northeast").Cells.EntireColumn.AutoFit
    Application.CutCopyMode = False
   Sheets("Surgical Backorders").Select
    ActiveSheet.Range("$A$1:$V$300000").AutoFilter Field:=14, Criteria1:=Array( _
        "4101", "4102", "4103", "4104", "4201", "4202", "4203", "4204", "4301", "4302", "4303", _
        "4304", "4401", "4402", "4403", "4404", "4501", "4502", "4503", "4504"), Operator:= _
        xlFilterValues
    Range("A1:V1", Selection.End(xlDown)).Copy
    Sheets("Southeast").Select
    Sheets("Southeast").Range("A1").PasteSpecial Paste:=xlPasteValues
    Sheets("Southeast").Range("A1").PasteSpecial Paste:=xlPasteFormats
    Sheets("Southeast").Cells.EntireColumn.AutoFit
    Application.CutCopyMode = False

etc etc etc for around 30 sheets.

I am still not sure what happened with my original solution though, and why it would only work in debug mode. I guess there are times when you have to use SELECT, in order for excel to catch up??


Thanks for the help. It looks like this forum will be a valuable resource for help!

 

[RoryA]

There are very few occasions when you need to select anything to use it in code and this certainly shouldn't be one of them. Your code is however still referring to selection even when it doesn't explicitly select anything. I'd suggest something like this (I assume you have code to turn screenupdating off already?)

    Dim wsBackOrders As Worksheet
    Dim rgSource As Range
    
    Set wsBackOrders = Sheets("Surgical Backorders")
    Set rgSource = wsBackOrders.Range("A1").CurrentRegion
    
    With rgSource
        .AutoFilter Field:=14, Criteria1:=Array("3102", "3103", "3104", _
            "3201", "3202", "3203", "3204", "3205", "3301", "3302", "3303", "3304", "3305", "3401", _
            "3402", "3403", "3404", "3501", "3502", "3503", "3504", "3505", "3601", "3602", "3603", _
            "3604", "3605"), Operator:=xlFilterValues
        If .Columns(1).SpecialCells(xlCellTypeVisible).Count > 1 Then
            .Copy
            With Sheets("Northeast")
                With .Range("A1")
                    .PasteSpecial Paste:=xlPasteValues
                    .PasteSpecial Paste:=xlPasteFormats
                End With
                Application.CutCopyMode = False
                .Cells.EntireColumn.AutoFit
            End With
        End If
        
        .AutoFilter Field:=14, Criteria1:=Array("4101", "4102", "4103", _
            "4104", "4201", "4202", "4203", "4204", "4301", "4302", "4303", _
            "4304", "4401", "4402", "4403", "4404", "4501", "4502", "4503", "4504"), _
            Operator:=xlFilterValues
        If .Columns(1).SpecialCells(xlCellTypeVisible).Count > 1 Then
            .Copy
            With Sheets("Southeast")
                With .Range("A1")
                    .PasteSpecial Paste:=xlPasteValues
                    .PasteSpecial Paste:=xlPasteFormats
                End With
                Application.CutCopyMode = False
                .Cells.EntireColumn.AutoFit
            End With
        End If
    End With

I've added a check in there to ensure there's actually something to copy before it proceeds.

 

[logandiana]

There are very few occasions when you need to select anything to use it in code and this certainly shouldn't be one of them. Your code is however still referring to selection even when it doesn't explicitly select anything. I'd suggest something like this (I assume you have code to turn screenupdating off already?)


I've added a check in there to ensure there's actually something to copy before it proceeds.

Thanks for the help. I tried this and it works perfectly. I understand why it works too!
But from my code that I fixed where I took out the SELECTS, You say that the code is still referring to the selection even when nothing is explicitly selected. How can that be possible? I still don't understand why it worked in debug mode, but not full out.

 

[Norie]

You are still using Select for the sheet(s) and you are also using ActiveSheet, what you should do is explicitly refer to the sheet as Rory is doing in.

Also, you are using Selection here, without any sheet reference.

  Range("A1:V1", Selection.End(xlDown)).Copy