VBA Script to Remove Duplicates

I'm curious if there's any way I can substantially improve its speed (it runs through >200k rows). I think I'll most likely need this once, so please bear with me since it probably looks terrible.

Code I would like reviewed:

Option Explicit

Public Sub RemoveDuplicates()
Dim r As Range
Dim rBefore As Range
Dim wsSource As Worksheet
Dim ws As Worksheet
Dim c As Range
Dim c_ As Range
Dim v As Variant
Dim v_ As Variant
Dim dupCount As Long
Dim xlApp As Application
Dim i As Long
Dim size As Long

Set xlApp = Excel.Application
With xlApp
.ScreenUpdating = False
.DisplayStatusBar = True
.StatusBar = "Running Script..."
End With

Set wsSource = xlApp.ThisWorkbook.Worksheets("source")
Set rBefore = wsSource.Range(wsSource.Cells(2, 1), wsSource.Cells(wsSource.UsedRange.Rows.Count, 1))

Set ws = xlApp.ThisWorkbook.Worksheets("testSheet")
Set r = ws.Range(ws.Cells(2, 1), ws.Cells(ws.UsedRange.Rows.Count, 1))
size = r.Count
i = 1
For Each v In r
Set c = v

DoEvents
dupCount = 0
For Each v_ In r
Set c_ = v_

DoEvents
If CStr(c.Value) = CStr(c_.Value) Then
dupCount = dupCount + 1
If dupCount > 1 Then
c_.Rows(c_.Row).EntireRow.Delete
End If
End If
Next
xlApp.StatusBar = Format(i & " out of " & size) & " Rows Complete"
i = i + 1
Next

With xlApp
.ScreenUpdating = True
.DisplayStatusBar = False
.ThisWorkbook.Save
.ThisWorkbook.Close
End With
End Sub

• Any reason why Range.RemoveDuplicates can't be used here? – Mathieu Guindon Aug 24 '18 at 17:48
• @MathieuGuindon None other than I didn't know about it until your comment; I'll check it out! – jcrizk Aug 24 '18 at 17:52

Seems Range.RemoveDuplicates could turn it all into a much more efficient one-liner.

Stylistically though, the code could use some help anyway.

The rather procedure begins with a good dozen variable declarations. Code is easier to follow when you don't need to constantly scroll up and down and back to know what's what: by declaring variables closer to their usage, you reduce the cognitive load and avoid that scrolling.

Dim xlApp As Application
Set xlApp = Excel.Application


Now, there's no need for this variable; Application is free, globally-scoped, and you're not owning that instance - just copying a reference to an existing object. Don't do that, use the globals that are right there waiting to be referenced instead. This includes any worksheet that exist at compile-time in ThisWorkbook:

Dim wsSource As Worksheet
Set wsSource = xlApp.ThisWorkbook.Worksheets("source")


Every worksheet has a CodeName property, that you can change in the properties toolwindow (F4). Locate that "source" worksheet in the Project Explorer, then look at its (Name) property - change it from what defaults to e.g. Sheet1, to a meaningful identifier, e.g. SourceSheet. And now you have a free, global-scope worksheet object referring to that specific sheet, readily usable everywhere in that VBA project - which makes this wsSource variable redundant as well.

Dim rBefore As Range
Set rBefore = wsSource.Range(wsSource.Cells(2, 1), wsSource.Cells(wsSource.UsedRange.Rows.Count, 1))


It's not clear whether that r stands for Range (in which case it's useless Hungarian Notation), and Before what this range is. So we try to look at other usages to infer its meaning... and we don't find any: the variable is assigned, but the assigned reference isn't used anywhere - the variable is redundant and can be removed entirely.

Dim ws As Worksheet
Set ws = xlApp.ThisWorkbook.Worksheets("testSheet")


Again, the CodeName could be TestSheet, and ws wouldn't be needed, provided that the worksheet exists at compile-time. Otherwise, kudos for fetching it from the Worksheets collection.

Dim r As Range
Set r = ws.Range(ws.Cells(2, 1), ws.Cells(ws.UsedRange.Rows.Count, 1))


UsedRange isn't a reliable way to get the last row of a column.

r isn't a meaningful name. testRange would tell us it's from the TestSheet, but wouldn't convey what it means - any single-letter variable name can never tell us anything whatsoever about something's purpose. Unless it's i for a For...Next loop counter, which any programmer knows about; here i isn't used like that, it's more like some processedRows.

DoEvents inside the loop, combined with updating the status bar at every single iteration, is contributing to slowing things down.

You're disabling ScreenUpdating, automatic Calculation, etc.; but you leave EnableEvents enabled, so a worksheet event is fired whenever you delete a row. You could use Union to combine all the rows you want to delete, and then delete them all in one single operation - instead of deleting them one at a time. But then again, Range.RemoveDuplicates makes that moot.

There are a number of issues (it's a constant work-in-progress), but Rubberduck could have picked up a number of the observations I made above. Kudos for Option Explicit, the meaningful name, and the explicit Public modifier on your macro!

• Stylistically, I know it's garbage, but there's a wealth of info NuGets that didn't even cross my mind. Thanks for the detailed answer! – jcrizk Aug 24 '18 at 21:45