-1

Can anyone tell me why the below code doesn't run any faster when i use an array? Seems to be taking 10 min or more to run, which is odd because yesterday it would take maybe a min or two without the array

Sub populateHRData9()

hrArray = Worksheets("HR_Report").Range("A1").CurrentRegion.Value
Set report = ActiveWorkbook.Worksheets("Report")


For x = 2 To report.UsedRange.Rows.Count
    If report.Cells(x, 1) = "" Then
    End If
Next x


For j = 2 To UBound(hrArray, 1)
    If hrArray(j, 42) <> "United Kingdom" Or hrArray(j, 42) <> "France" Or 
hrArray(j, 42) <> "Ireland" Or hrArray(j, 42) <> "Italy" Or hrArray(j, 42) <> 
"Netherlands" Or hrArray(j, 42) <> "Spain" Or hrArray(j, 42) <> "Sweden" Or 
hrArray(j, 42) <> "Germany" Or hrArray(j, 42) <> "Austria" Then
    If hrArray(j, 10) = "WEALTH MANAGEMENT" Or hrArray(j, 10) = "INTELLIGENT 
DIGITAL SOLUTIONS" Then
    If hrArray(j, 18) = "Administrative Asst - AM Investors/WM Solutions" Or 
hrArray(j, 18) = "Administrative Asst (Sales Service)" Or hrArray(j, 18) = 
"Administrative Asst (Sales/CA Support)" Or hrArray(j, 18) = "Client Service 
Total" Or hrArray(j, 18) = "Communications" Or hrArray(j, 18) = "Fiduciatary" 
Or hrArray(j, 18) = "Front Office Interns" Or hrArray(j, 18) = "Investments" 
Or hrArray(j, 18) = "Investors Service - ex Program Analysts" Or hrArray(j, 
18) = "JPMS Financial Advisors" Or hrArray(j, 18) = "JPMS Solutions" Or 
hrArray(j, 18) = "Marketing and Events" Or hrArray(j, 18) = "Mortgage 
Advisory" Or hrArray(j, 18) = "Origination/Client Manager" Or hrArray(j, 18) 
= "Other" Or hrArray(j, 18) = "Solutions - Program Analyst" Or hrArray(j, 18) 
= "Summer Intern" Or hrArray(j, 18) = "Supervisory Management" Or hrArray(j, 
18) = "WM Bankers" Or hrArray(j, 18) = "WM Capital Advisors" _
    Or hrArray(j, 18) = "WM Investors" Or hrArray(j, 18) = "WM MM/RH/PL" Or 
hrArray(j, 18) = "WM Prosectors" Or hrArray(j, 18) = "WM Trusts Officers" Or 
hrArray(j, 18) = "WM Wealth Advisors" Or hrArray(j, 18) = "WMOC" Then
    report.Cells(x, 1) = hrArray(j, 3)


 x = x + 1
    End If
    End If
    End If

Next j

End Sub

Excuse the multiple if statements. If was skipping "United Kingdom" when i used an or. Here is the code i had before and actually after restarted the computer it is fast again.

Sub populateHRData9()


Set report = ActiveWorkbook.Worksheets("Report")
Set hr = ActiveWorkbook.Worksheets("HR_Report")


For x = 2 To report.UsedRange.Rows.Count
    If report.Cells(x, 1) = "" Then
    End If
Next x


For j = 2 To hr.UsedRange.Rows.Count
    If hr.Cells(j, 42) <> "United Kingdom" Then
    If hr.Cells(j, 42) <> "France" Then
    If hr.Cells(j, 42) <> "Ireland" Then
    If hr.Cells(j, 42) <> "Italy" Then
    If hr.Cells(j, 42) <> "Netherlands" Then
    If hr.Cells(j, 42) <> "Spain" Then
    If hr.Cells(j, 42) <> "Sweden" Then
    If hr.Cells(j, 42) <> "Germany" Then
    If hr.Cells(j, 42) <> "Austria" Then
    If hr.Cells(j, 10) = "WEALTH MANAGEMENT" Or hr.Cells(j, 10) = 
"INTELLIGENT DIGITAL SOLUTIONS" Then
    If hr.Cells(j, 18) = "Administrative Asst - AM Investors/WM Solutions" 
Or hr.Cells(j, 18) = "Administrative Asst (Sales Service)" Or hr.Cells(j, 
18) = "Administrative Asst (Sales/CA Support)" Or hr.Cells(j, 18) = "Client 
Service Total" Or hr.Cells(j, 18) = "Communications" Or hr.Cells(j, 18) = 
"Fiduciatary" Or hr.Cells(j, 18) = "Front Office Interns" Or hr.Cells(j, 18) 
= "Investments" Or hr.Cells(j, 18) = "Investors Service - ex Program 
Analysts" Or hr.Cells(j, 18) = "JPMS Financial Advisors" Or hr.Cells(j, 18) 
= "JPMS Solutions" Or hr.Cells(j, 18) = "Marketing and Events" Or 
hr.Cells(j, 18) = "Mortgage Advisory" Or hr.Cells(j, 18) = 
"Origination/Client Manager" Or hr.Cells(j, 18) = "Other" Or hr.Cells(j, 18) 
= "Solutions - Program Analyst" Or hr.Cells(j, 18) = "Summer Intern" Or 
hr.Cells(j, 18) = "Supervisory Management" Or hr.Cells(j, 18) = "WM Bankers" 
Or hr.Cells(j, 18) = "WM Capital Advisors" _
    Or hr.Cells(j, 18) = "WM Investors" Or hr.Cells(j, 18) = "WM MM/RH/PL" 
Or hr.Cells(j, 18) = "WM Prosectors" Or hr.Cells(j, 18) = "WM Trusts 
Officers" Or hr.Cells(j, 18) = "WM Wealth Advisors" Or hr.Cells(j, 18) = 
"WMOC" Then
    report.Cells(x, 1) = hr.Cells(j, 3)


 x = x + 1
    End If
    End If
    End If
    End If
    End If
    End If
    End If
    End If
    End If
    End If
    End If

Next j

End Sub
Community
  • 1
  • 1
Josh Hudson
  • 103
  • 1
  • 10
  • 2
    This would be much cleaner (and maybe faster) using `Select Case` – Marcucciboy2 Jun 26 '18 at 16:12
  • Is setting (for example) the value of `hrArray(j, 10)` dependent on `hrArray(j, 42)`? I think the way that you've set up your `If` statements might be causing you all the trouble - I believe you should be using `End If` before you start the next `If` – Marcucciboy2 Jun 26 '18 at 16:16
  • Yes, basically it has to meet the requirement of each if statement. Im not familiar with select case. so if something does not equal "United Kingdom" and equals "Wealth Management" and equals "Front Office Interns" then enter their ID on the list. It was working fine yesterday.. im not sure what happened today This is referencing one worksheet and returning the matched ID's onto another – Josh Hudson Jun 26 '18 at 16:19
  • CurrentRegion or UsedRange might be much larger than you think it is. The `For x = 2 To report.UsedRange.Rows.Count` is probably woefully inefficient for whatever you're using it for. None of your variables have been declared, which increases the overhead as the compiler needs to type them at runtime rather than at compile time, etc. There is soooo much going on here that it's hard to offer any constructive solution to what is, ultimately, a pretty vague problem. – David Zemens Jun 26 '18 at 17:45

1 Answers1

0

Okay, so as I thought about this, I realized that you were essentially using arrays backwards - I don't think it makes it faster for you to store those initial values in an array. I believe the part that you really want to be an array is the values that you're checking against, so that's what I did to them!

This is untested so let me know if it works/is faster.

Sub populateHRData9()

    Dim hrArray() As Variant
    hrArray = Worksheets("HR_Report").Range("A1").CurrentRegion.value

    Dim report As Worksheet
    Set report = ActiveWorkbook.Worksheets("Report")

    Dim lastRow As Long
    lastRow = report.Cells(report.Rows.count, "A").End(xlUp).row


    pt1 = Array("United Kingdom", "France", "Ireland", "Italy", "Netherlands", "Spain", "Sweden", "Germany", "Austria")

    pt2 = Array("WEALTH MANAGEMENT", "INTELLIGENT DIGITAL SOLUTIONS")

    pt3 = Array("Administrative Asst - AM Investors/WM Solutions", "Administrative Asst (Sales Service)", "Administrative Asst (Sales/CA Support)", _
        "Client Service Total", "Communications", "Fiduciatary", "Front Office Interns", "Investments", "Investors Service - ex Program Analysts", _
        "JPMS Financial Advisors", "JPMS Solutions", "Marketing and Events", "Mortgage Advisory", "Origination/Client Manager", _
        "Other", "Solutions - Program Analyst", "Summer Intern", "Supervisory Management", "WM Bankers", "WM Capital Advisors", "WM Investors", _
        "WM MM/RH/PL", "WM Prosectors", "WM Trusts Officers", "WM Wealth Advisors", "WMOC")

    Dim j As Long
    For j = 2 To UBound(hrArray, 1)

        If Not IsInArray(hrArray(j, 42), pt1) _
        And IsInArray(hrArray(j, 10), pt2) _
        And IsInArray(hrArray(j, 18), pt3) Then
            report.Cells(lastRow, 1) = hrArray(j, 3)
            lastRow = lastRow + 1
        End If
    Next j

End Sub

Function IsInArray(strIn As String, arrList() As Variant) As Boolean
    IsInArray = Not (IsError(Application.Match(strIn, arrList, 0)))
End Function
Marcucciboy2
  • 3,156
  • 3
  • 20
  • 38
  • Yes, so there are multiple subs run one after another. The x makes sure that the list being created starts at the end of the list each time... so after sub1() is done sub2() runs and first looks for the first empty row on the report tab to continue the list. – Josh Hudson Jun 26 '18 at 16:39
  • in that case, a simple way to find the last "filled" row is like so: `lastRow = Cells(Rows.count, "A").End(xlUp).row` – Marcucciboy2 Jun 26 '18 at 16:46
  • i appreciate it, but i dont believe that is where the slow down is happening.. Im just really confused because this worked so much quicker yesterday and all i did was add the array and remove the sheet reference – Josh Hudson Jun 26 '18 at 16:52
  • @JoshHudson yeah I agree i'm taking a look at other options to check faster in these types of scenarios. Would you mind including your previous code in your question, though? – Marcucciboy2 Jun 26 '18 at 16:56
  • added my code.. restarted the comp and my old code is fast again – Josh Hudson Jun 26 '18 at 17:11
  • ok to add... restarted the computer old code ran fine. replaced with array again and it took forever. didnt save changes and went to run old code and its crazy slow again. i think i give up lol – Josh Hudson Jun 26 '18 at 17:23
  • @JoshHudson okay i've changed my approach here, give this one a try and see if it works correctly – Marcucciboy2 Jun 26 '18 at 17:27
  • @JoshHudson if rebooting is having that sort of effect, you're having a hardware problem, not a code problem. Even if there are ways to optimize your code, the symptoms you're describing don't seem related to the code itself. If it works fine now, but later it's slow, and the code hasn't change, the code isn't the problem. The available resources on the machine executing it is the problem. – David Zemens Jun 26 '18 at 17:30
  • Also, [Application.Match() is up to ten times slower than calling a function which uses a loop.](https://stackoverflow.com/questions/18754096/matching-values-in-string-array). – David Zemens Jun 26 '18 at 17:32
  • ok so it seems to be that im writing into a table.... if i do it outside the table no problem, inside it takes forever. Also @Marcucciboy2 code did not work, i received a ByRef Argument type mismatch error – Josh Hudson Jun 26 '18 at 17:32
  • @JoshHudson every time you write a value to a sheet, unless you've explicitly disabled it, that triggers a *calculation* event as well as any event handlers you might have implemented on the worksheet in question (e.g.,`Change`, etc.). – David Zemens Jun 26 '18 at 17:36
  • @DavidZemens yikes, i'm considering deleting this frankenstein of an answer as it currently doesn't serve a purpose but to confuse. – Marcucciboy2 Jun 26 '18 at 17:39
  • TBH I'd vote to close this question as either off-topic or too localized. This isn't really a very good question. The problem is too specific to be broadly helpful to others. – David Zemens Jun 26 '18 at 17:41