Fast(er) web scraping with VBA

Question

I have a code that fetches rates from a website called X-Rates, and outputs to excel the monthly averages of a chosen country.

The code runs quite fast, but I still think I could improve the code a little bit, but just don't know what to look for. I've done the obvious things like, making the option explicit and disabling screen updating. Can someone point my flaws?

Also you will see that the code uses if's instead of select case. Could this be an improvement to think about atm?

Apologies for long code, but if you help me I would be eternally grateful!

Option Explicit
Sub fetchCurrencyPast()
'Define variables
Dim a            As Integer
Dim b            As Integer
Dim c            As Integer
Dim d            As Integer
Dim i            As Integer
Dim boolCtrl     As Boolean
Dim period       As Variant
Dim sCrcy        As Variant
Dim MsgErr       As String


'Error handler
On Error GoTo ErrHandler

'Apply format text on cells, and centre it.
'Change format to text

Columns("A:F").Select
With Selection
    .HorizontalAlignment = xlCenter
    .NumberFormat = "@"
End With

'Clear selection
Cells(1, 1).Select

'Add header
Range("A1", "F1").Style = "Input"
Range("A1", "F1").Font.Bold = True
Cells(1, 1).Value = "Year"
Cells(1, 2).Value = "OffSetCurr"
Cells(1, 3).Value = "Month"
Cells(1, 4).Value = "toEuro"
Cells(1, 5).Value = "toDollars"
Cells(1, 6).Value = "toPounds"


'Define flag for error
boolCtrl = False

'Define date and format as date
period = Application.InputBox("What's the year you want to collect back data?", "Period", , , , , 2)

On Error GoTo ErrHandler
'Error control on period
If Len(period) <> 4 Then
    boolCtrl = True
    GoTo ErrHandler
    Exit Sub
End If

'Make the code faster
Application.ScreenUpdating = False

'Start fetching values from each country
For i = 1 To 9

    'Define start row
    a = 2
    c = 2
    'Define start col
    b = 4
    d = 3

    If i = 1 Then
        'ARS
        Cells(a, 2).Value = "ARS"
        Cells(a, 1).Value = period

        For Each sCrcy In Array("EUR", "USD", "GBP")
            Call GetRateYear("ARS", sCrcy, period, a, b)
            a = 2
            b = b + 1
            c = a
            Call GetSingleMonth("ARS", sCrcy, period, c, d)
        Next

    End If

    If i = 2 Then
        a = 14
        b = 4
        'AUD
        Cells(a, 2).Value = "AUD"
        Cells(a, 1).Value = period

        For Each sCrcy In Array("EUR", "USD", "GBP")
            Call GetRateYear("AUD", sCrcy, period, a, b)
            a = 14
            b = b + 1
            c = a
            Call GetSingleMonth("AUD", sCrcy, period, c, d)
        Next
    End If

  'Other ifs afterwards for each country

Error Handler:

ErrHandler:

If Err.Number <> 0 Then

    MsgErr = "Error #" & Str(Err.Number) & " was generated by " & Err.Source & "." & Chr(13) & "Error description: " & Err.Description
    MsgBox MsgErr, , "Error", Err.HelpFile, Err.HelpContext
    Exit Sub
End If

If boolCtrl = True Then
    MsgBox "Wrong date. Please retry!", vbCritical + vbOKOnly, "Error found!"
End If
End Sub

Function GetRateYear :

Function GetRateYear(sFromCrcy, sToCrcy, sYear, a, b)

'This function sends a XML HTTP request, as is much more faster than waiting for browser to DoEvents
'Usage -> Goes to X-rates website and retrieves the code from conversion

Dim sUrl, sContent, intMatches
Dim mtchCnt      As Integer
Dim subMtchCnt   As Integer


sUrl = "http://www.x-rates.com/average/?from=" & sFromCrcy & "&to=" & sToCrcy & "&amount=1&year=" & sYear

'XML Object that queries the website and retrieves HTML as text
With CreateObject("MSXML2.XMLHttp")
    .Open "GET", sUrl, False
    .send
    sContent = .responseText
End With

'This retrieves values of currency (until end with)
With CreateObject("VBScript.RegExp")
    .Global = True
    .MultiLine = True
    .IgnoreCase = False
    .Pattern = "<span class=""avgRate"">(.*?)</span>"

    'To do the count, you must always execute the regex first
    Set intMatches = .Execute(sContent)

    If intMatches.Count <> 0 Then
        With intMatches
            For mtchCnt = 0 To .Count - 1
                For subMtchCnt = 0 To .Item(subMtchCnt).SubMatches.Count - 1
                    GetRateYear = .Item(mtchCnt).SubMatches(0)
                    Cells(a, b).Value = GetRateYear
                    Cells(a, 1).Value = sYear
                    Cells(a, 2).Value = sFromCrcy
                    a = a + 1
                Next
            Next
        End With
    End If
  End With
End Function

Function GetSingleMonth

Function GetSingleMonth(sFromCrcy, sToCrcy, sYear, c, d)

'This function sends a XML HTTP request, as is much more faster than waiting for browser to DoEvents
'Usage -> Goes to X-rates website and retrieves the code from conversion

Dim sUrl, sContent, intMatches
Dim mtchCnt2 As Integer
Dim subMtchCnt2  As Integer

sUrl = "http://www.x-rates.com/average/?from=" & sFromCrcy & "&to=" & sToCrcy & "&amount=1&year=" & sYear

'XML Object that queries the website and retrieves HTML as text
With CreateObject("MSXML2.XMLHttp")
    .Open "GET", sUrl, False
    .send
    sContent = .responseText
End With

'This retrieves values of currency (until end with)
With CreateObject("VBScript.RegExp")
    .Global = True
    .MultiLine = True
    .IgnoreCase = False
    .Pattern = "<span class=""avgMonth"">(.*?)</span>"

    'To do the count, you must always execute the regex first
    Set intMatches = .Execute(sContent)

    If intMatches.Count <> 0 Then
        With intMatches
            For mtchCnt2 = 0 To .Count - 1
                GetSingleMonth = .Item(mtchCnt2).SubMatches(0)
                Cells(c, d).Value = GetSingleMonth
                c = c + 1
            Next
        End With
    End If
  End With
End Function

Answer 1

Good job on your question.

It's good practice to indent all of your code that way Labels will stick out as obvious.

---

Dim sUrl, sContent, intMatches

When you don't define your variable, VBA will declare it as a Variant, which are objects:

Performance. A variable you declare with the Object type is flexible enough to contain a reference to any object. However, when you invoke a method or property on such a variable, you always incur late binding (at run time). To force early binding (at compile time) and better performance, declare the variable with a specific class name, or cast it to the specific data type.

By not declaring variables, you could possibly be paying a penalty.

---

Dim a            As Integer
Dim b            As Integer
Dim c            As Integer
Dim d            As Integer
Dim i            As Integer
Dim boolCtrl     As Boolean
Dim period       As Variant
Dim sCrcy        As Variant
Dim MsgErr       As String

Variable names - give your variables meaningful names.

Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.

boolCtrl - no need for bool

Hungarian naming? Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

---

With Selection
    .HorizontalAlignment = xlCenter
    .NumberFormat = "@"
End With

Be sure to avoid things like .Select - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this - https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros .

---

'This retrieves values of currency (until end with)

Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

---

Function GetRateYear(sFromCrcy, sToCrcy, sYear, a, b)

If possible, you should pass argument ByVal rather than ByRef. ByRef is default.

Your functions should be Private instead of Public. Public is default.

Functions should be used when something is returned and subs should be used when something happens.

Private Function GetRateYear(ByVal fromCurrency as String, ByVal toCurrency as String, ByVal year as String, ByVal a as Long, ByVal b as Long) As ??

Answer 2

Nitpick, but Sub fetchCurrencyPast() should be Public Sub FetchCurrencyPast(), i.e. using PascalCasing consistent with the rest of module members, and explicitly Public for clarity; VBA does so many things behind our backs, it's good to always be explicit where possible!

The first executable statement in that procedure makes an important assumption:

Columns("A:F").Select

The Columns call is unqualified, which means it operates on whichever worksheet is active when the code runs. Worse, every single worksheet-accessing statement in the procedure is also implicitly referring to the active sheet, which means if the user manages to activate another sheet while the code is running, the macro will start outputting to that newly activated sheet!

If you're always working off Sheet1, then you can use that object reference to qualify these calls, so that no matter what the user does while the code is running, it will always be working with the same Sheet1 object reference:

Sheet1.Columns("A:F").Select

Ideally you would give that worksheet a meaningful name, e.g. if its tab is named "Results", you could set the sheet's code name to ResultSheet, and refer to it like this in code:

ResultSheet.Columns("A:F").Select

Now, Select, Activate, and working with the Selection aren't particularly efficient ways of working with a worksheet. It's typically what less experienced VBA programmers do though, because that's how the macro recorder works.

Why are we selecting columns A:F here?

'Apply format text on cells, and centre it.
'Change format to text

Columns("A:F").Select
With Selection
    .HorizontalAlignment = xlCenter
    .NumberFormat = "@"
End With

This could be abstracted into its own procedure:

Private Sub FormatResultSheet()

    Dim target As Range
    Set target = ResultSheet.Range("A:F")

    target.HorizontalAligment = xlCenter
    target.NumberFormat = "@" 'otherwise Excel trims leading zeroes

End Sub

Comments that say what the code is doing, are useless and should be removed. The only comments worth typing, are comments that say why code is doing what it does (see example above).

---

This isn't needed at all:

'Clear selection
Cells(1, 1).Select

Your code shouldn't be bothered with the current selection.

---

I won't repeat the excellent points in @Raystafarian's answer about this block:

'Define variables
Dim a            As Integer
Dim b            As Integer
Dim c            As Integer
Dim d            As Integer
Dim i            As Integer
Dim boolCtrl     As Boolean
Dim period       As Variant
Dim sCrcy        As Variant
Dim MsgErr       As String

But I'll add mine:

• Again, comments: anybody can see that these are variable declarations; a comment that says "hey look that's a bunch of variables!" is just a distraction.
• Don't align types like this. It's a sheer waste of time and a royal pain to maintain whenever you rename one single variable.
• Declare variables closer to their usage. "Oh but I like seeing all the variables I'm using in a procedure in one place" isn't a valid argument against that guideline. Code objectively reads more fluently when you're not constantly referring back to that list of variables at the top of the procedure. And it's even worse when there's a dozen of them (or more!), and worse even when the procedure does so many things you constantly need to scroll up and down.

---

Chr(13) gives you one of the two characters Windows uses to encode new lines. The other character is Chr(10).

.." & Chr(13) & "..

A more portable (you know, if you ever want to run that macro on a Mac) and generally more readable way of doing this, is to use the built-in vbNewLine constant, which generates whatever "new line" characters the OS likes to use.

---

The more I read, the more it seems you're using comments to say "this chunk of code does X":

'Add header
Range("A1", "F1").Style = "Input"
Range("A1", "F1").Font.Bold = True
Cells(1, 1).Value = "Year"
Cells(1, 2).Value = "OffSetCurr"
Cells(1, 3).Value = "Month"
Cells(1, 4).Value = "toEuro"
Cells(1, 5).Value = "toDollars"
Cells(1, 6).Value = "toPounds"

Every time you have a comment that says "This chunk does (something)", you have a missed opportunity (it's never too late though!) of extracting a private method that does that thing.

Private Sub AddHeaders()
    With ResultSheet
        .Range("A1", "F1").Style = "Input"
        .Range("A1", "F1").Font.Bold = True
        .Cells(1, 1).Value = "Year"
        .Cells(1, 2).Value = "OffSetCurr"
        .Cells(1, 3).Value = "Month"
        .Cells(1, 4).Value = "toEuro"
        .Cells(1, 5).Value = "toDollars"
        .Cells(1, 6).Value = "toPounds"
    End With
End Sub