0

**Thanks all for the pointers on how to as well as the Code Review section. Today I switched over to pulling and comparing the numbers by making an array for each group of numbers. It works in seconds now rather than minutes. **

I have working code, it does the job just fine. It's purpose is to check and report if there are any duplicate loan numbers by comparing the ReadyForExport (normally about 60 rows) sheet to the PastLoanLog sheet (presently about 1300 rows) one by one.

Question: Any ideas on how to code this better? It takes a few minutes to run, but if there is a way I can make it run faster, that's what I am searching for. Here is the code:

Sub DupTest2()

'This runs through the RFE list, checks the 2nd mortgage numbers
'and reviews against the PastLoanLog spreadsheet

MsgBox ("This may take a minute")

OpenSheets 'Opens worksheets needed to run the program

Dim TestDpaNum As String
Dim PastDpaNum As String
Dim lRow As Integer
Dim DupNum As Integer
Dim h As Integer
Dim i As Integer
Dim lrowHFE As Integer

Sheets("ReadyForExport").Select
Range("G2").Select
lrowHFE = Cells(Rows.Count, 1).End(xlUp).Row
Debug.Print "Ready For Export LR " & lrowHFE


'Locate Last Row In PastLoanLog Data
'**********************************
Sheets("PastLoanLog").Select
Range("G2").Select
lRow = Cells(Rows.Count, 1).End(xlUp).Row


Sheets("ReadyForExport").Select
Range("G2").Select

    For h = 2 To lrowHFE

    'Finds the first loan number to check against the old data

    TestDpaNum = ActiveCell.Value

    Sheets("PastLoanLog").Select

    Range("G2").Select

            For i = 1 To lRow

            'Selects current cell to compare with cell from RFE sheet
            PastDpaNum = ActiveCell.Value

                If PastDpaNum = TestDpaNum Then
                    DupNum = DupNum + 1
                    Debug.Print "Duplicate Found" & TestDpaNum
                    Sheets("ErrorSheet").Range(DupNum, 6).Value = TestDpaNum
                    ActiveCell.Offset(1, 0).Select

                Else
                    ActiveCell.Offset(1, 0).Select

                End If

            Next

    Sheets("ReadyForExport").Select
    ActiveCell.Offset(1, 0).Select
    Debug.Print "CurrentRow=" & h

Next

'Sends the info to the Dashboard

Debug.Print "Dups = " & DupNum
Sheets("Dashboard").Select
Range("P16").Select
ActiveCell.Value = DupNum
ActiveCell.Offset(1, 0).Value = Now()
CloseSheets

End Sub
Diet_Rich
  • 15
  • 2
  • 6
  • 5
    This might be a better fit for Code Review, but you can start by rewriting to get rid of all of your `Select` methods. There's an [article here](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) on SO as to why they are bad, and how to get rid of them. – Ron Rosenfeld Nov 05 '18 at 19:52
  • @RonRosenfeld's comment is very on-point. As a band-aid you can turn off screenupdating and set calculation to manual while your loop is running (don't forget to set calculation back to automatic before your code exits...) – Tim Williams Nov 05 '18 at 20:08
  • 1
    I would recommend to use a `Dictionary` (by adding a reference to `Microsoft Scripting Runtime`) fill it once (instead of performing a linear search for each number) and then just use Dictionary 's `exist` method to check if the loan number already occured. This should speed up the execution and will require less code. – Maksim Nov 05 '18 at 21:34
  • Thanks for pointing out Code Review. I'll use that for future projects. As a followup I didn't dig into learning Dictionaries, though it looked promising. Instead I made each set of numbers into their own Array, and then compared them. Pretty much runs just like above but there is no Select statements. Runs fast now. Thank you all. – Diet_Rich Jan 14 '19 at 22:24

0 Answers0