HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Fast(er) web scraping with VBA

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fastwithscrapingwebvba

Problem

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 = p

Solution

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 ??

Code Snippets

Dim sUrl, sContent, intMatches
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
boolCtrl - no need for bool
With Selection
    .HorizontalAlignment = xlCenter
    .NumberFormat = "@"
End With
'This retrieves values of currency (until end with)

Context

StackExchange Code Review Q#128160, answer score: 4

Revisions (0)

No revisions yet.