Macro that does some kind of duplicate check

Question

This macro pulls in two workbooks, one being a template with saved formulas already, and the other containing data with thousands of rows...I need to increase the speed because the process takes more than 15 minutes.

Sub WbtoWb4()

Dim Wb1 As Workbook
Dim Wb2 As Workbook

With Application
.ScreenUpdating = False
.EnableEvents = False
.DisplayAlerts = False
End With
Set Wb1 = Workbooks.Open("")
Set Wb2 = Workbooks.Open("")

Wb1.Sheets("CDGL Data").Copy After:=Wb2.Sheets("STS")
Wb1.Close False

With Application
.ScreenUpdating = True
.EnableEvents = True
.DisplayAlerts = True
End With

Sheets("CDGL Data").Select
Range("AQ:BB").EntireColumn.Delete

Range("A1").AutoFilter Field:=32, Criteria1:=Sheets("DataSources").Range("B4").Value
ActiveSheet.UsedRange.Offset(1, 0).SpecialCells _
(xlCellTypeVisible).Copy

Sheets("CDGL").Select
Range("B2").PasteSpecial Paste:=xlPasteValues

With Sheets("CDGL")
rows_c1 = .Cells(Rows.Count, "G").End(xlUp).Row
Sheets("Duplicate Check").Range("A1:C" & rows_c1).Value = Sheets("CDGL").Range("H2:J" & rows_c1).Value

rows_c2 = .Cells(Rows.Count, "K").End(xlUp).Row
Sheets("Duplicate Check").Range("D1:G" & rows_c2).Value = Sheets("CDGL").Range("L2:O" & rows_c2).Value

rows_c3 = .Cells(Rows.Count, "AI").End(xlUp).Row
Sheets("Duplicate Check").Range("H1:H" & rows_c3).Value = Sheets("CDGL").Range("AJ2:AJ" & rows_c3).Value
End With

Sheets("Duplicate Check").Select
Set rng = Range("A1", Range("H1").End(xlDown))
rng.RemoveDuplicates Columns:=Array(1, 2, 3, 4, 5, 6, 7, 8), Header:=xlNo

With Sheets("CDGL")
Sheets("Rec").Range("B6").Resize(.Cells(.Rows.Count, "G").End(xlUp).Row - 1, 3).Value = Sheets("Duplicate Check").Range("A1:C" & .Cells(.Rows.Count, "A").End(xlUp).Row).Value
 Sheets("Rec").Range("E6").Resize(.Cells(.Rows.Count, "D").End(xlUp).Row - 1, 4).Value = Sheets("Duplicate Check").Range("D1:G" & .Cells(.Rows.Count, "A").End(xlUp).Row).Value
 Sheets("Rec").Range("I6").Resize(.Cells(.Rows.Count, "H").End(xlUp).Row - 1, 1).Value = Sheets("Duplicate Check").Range("H1:H" & .Cells(.Rows.Count, "A").End(xlUp).Row).Value
End With

Application.DisplayAlerts = False
Sheets("Duplicate Check").Delete

ActiveWorkbook.SaveAs Filename:=""
ActiveWorkbook.Close
End Sub

Answer 1

Lowest-Hanging VBA Performance Fruit

You have

With Application
.ScreenUpdating = False
.EnableEvents = False
.DisplayAlerts = False
End With

But you only have it in force for part of your Sub. If you move this

With Application
.ScreenUpdating = True
.EnableEvents = True
.DisplayAlerts = True
End With

To the very end of your Sub, you should instantly see a big improvement.

In addition, you should set Application.Calculate to xlCalculationManual at the start, and set it back to xlCalculationAutomatic at the end. That's the other big performance drain.

---

Always Reset anything you disable

Whenever you change an Application.Thing setting, that change will persist as long as Excel remains open. This:

Application.DisplayAlerts = False

is never set back to True. Once your Sub ends, if your user accidentally clicks the close button, Excel will close without prompting them to save their work first. Because alerts are disabled.

Answer 2

There are a variety of clean up things you can do first, because the code is confusing in what exactly you're trying to do.

1. Always use Option Explicit. It will help you avoid confusion in how variables are used.
2. Some redundant/similar code blocks should be broken out into helper functions:

Private Sub SetUpdates(ByVal newState As Boolean)
    With Application
        .ScreenUpdating = newState
        .EnableEvents = newState
        .DisplayAlerts = newState
    End With
End Sub

3. Create variable names that reflect what you're trying to do. It will help you avoid confusing which is your source and which is your destination. This includes helping to keep track of the data on different Worksheets. For example:

Sub WbtoWb4()
    Dim srcWB As Workbook
    Dim srcWS As Worksheet
    Dim dstWB As Workbook
    Dim cdglDataWS As Worksheet
    Dim cdglWS As Worksheet
    Dim dupCheckWS As Worksheet
    Dim lastRow as Long

    '--- disables screen updates and events
    SetUpdates newState:=False

    '--- establish link to source data
    Set srcWB = Workbooks.Open("")          'missing filename?
    Set srcWS = srcWB.Sheets("CDGL Data")

    '--- establish link to destination
    Set dstWB = Workbooks.Open("")          'missing filename?
    srcWS.Copy After:=dstWB.Sheets("STS")
    srcWB.Close SaveChanges:=False

    Set cdglDataWS = dstWB.Sheets("CDGL Data")
    Set cdglWS = dstWB.Sheets("CDGL")
    Set dupCheckWS = dstWB.Sheets("Duplicate Check")

...

4. Use comments to describe the actions for a block of code. For example, it will be important to you later to understand WHY you're deleting all those columns.

    '--- state "why" you're deleting all these columns and then filtering
    With cdglWS
        .Range("AQ:BB").EntireColumn.Delete
        .Range.AutoFilter Field:=32, _
                    Criteria1:=dstWB.Sheets("DataSources").Range("B4").Value
        .UsedRange.Offset(1, 0).SpecialCells(xlCellTypeVisible).Copy
    End With

5. Your variables rows_c1, rows_c2, and rows_c3 are only ever used once. Consider re-using a single variable, e.g. lastRow.
6. You may find some speed improvements if you create a Range object for each source/destination when you copy, instead of forcing VBA to interpret multiple steps for each line. By separating the definition of the ranges, you'll certainly have an easier time debugging later. For example:

    '--- what data are you copying with these actions?
    lastRow = .Cells(Rows.Count, "G").End(xlUp).Row
    Set toRange = dupCheckWS.Range("A1").Resize(lastRow, 3)
    Set fromRange = cdglWS.Range("H2").Resize(lastRow, 3)
    toRange.Value = fromRange.Value

7. It's possible the RemoveDuplicates function (an Excel built-in) may be the slowest point in your code. To really get a sense of where your slow-down is, you may have to profile your code with performance timers. However, @Zak 's suggestion to move the re-enabling of ScreenUpdates and such to the end of your method will likely do the trick (or at least give you a much better performance than you have now).

Answer 3

For simple readability, you can change this:

With Sheets("CDGL")
rows_c1 = .Cells(Rows.Count, "G").End(xlUp).Row
Sheets("Duplicate Check").Range("A1:C" & rows_c1).Value = Sheets("CDGL").Range("H2:J" & rows_c1).Value

rows_c2 = .Cells(Rows.Count, "K").End(xlUp).Row
Sheets("Duplicate Check").Range("D1:G" & rows_c2).Value = Sheets("CDGL").Range("L2:O" & rows_c2).Value

rows_c3 = .Cells(Rows.Count, "AI").End(xlUp).Row
Sheets("Duplicate Check").Range("H1:H" & rows_c3).Value = Sheets("CDGL").Range("AJ2:AJ" & rows_c3).Value
End With

to this:

With Sheets("CDGL")
  rows_c1 = .Cells(Rows.Count, "G").End(xlUp).Row
  Sheets("Duplicate Check").Range("A1:C" & rows_c1).Value = .Range("H2:J" & rows_c1).Value

  rows_c2 = .Cells(Rows.Count, "K").End(xlUp).Row
  Sheets("Duplicate Check").Range("D1:G" & rows_c2).Value = .Range("L2:O" & rows_c2).Value

  rows_c3 = .Cells(Rows.Count, "AI").End(xlUp).Row
  Sheets("Duplicate Check").Range("H1:H" & rows_c3).Value = .Range("AJ2:AJ" & rows_c3).Value
End With

Note the 2 changes:

1. Indention of the rows within the With block makes it much easier to find where the code block ends

2. Since you're using With Sheets("CDGL"), there's no need to specify Sheets("CDGL") on each of the assignment rows that follow. This shortens the rows and makes them easier to read.

I'm not sure about VBA processing speed on With statements, and since you're not looping over it, it may not make a huge difference, but simply removing the With block (like this):

rows_c1 = .Cells(Rows.Count, "G").End(xlUp).Row
Sheets("Duplicate Check").Range("A1:C" & rows_c1).Value = Sheets("CDGL").Range("H2:J" & rows_c1).Value

rows_c2 = .Cells(Rows.Count, "K").End(xlUp).Row
Sheets("Duplicate Check").Range("D1:G" & rows_c2).Value = Sheets("CDGL").Range("L2:O" & rows_c2).Value

rows_c3 = .Cells(Rows.Count, "AI").End(xlUp).Row
Sheets("Duplicate Check").Range("H1:H" & rows_c3).Value = Sheets("CDGL").Range("AJ2:AJ" & rows_c3).Value

might make a slight speed improvement. Since you'd already given the full Sheet specification, there was no need to include it in a With structure in the first place.

Finally, a precautionary note: You're using unqualified Sheets() which will reference the currently active workbook, whatever it may be. Since you indicate that this process is taking 15+ minutes, it's possible that someone may click on another Excel workbook or open a new one, and your processing will immediately start acting on whatever workbook now has the focus in Excel. Explicitly assigning a workbook/worksheet to a worksheet variable, as described by PeterT in his 3rd point will prevent your code from accidentally acting on a different workbook/worksheet.