Financial Data From Webqueries in Excel

I'm new (to CR and to programming in general). I wrote my first VBA on Monday. This is my first working project. It Takes a bunch of financial data from a company called Financial Analytics and a bunch more from a number of webqueries, and puts it all in a nicely formatted table for external consumption.

It's a kludgy mess of stuff I've managed to make work, but I really have no idea what I'm doing. I imagine there'll be a lot of things to recommend so please feel free to only pick out a few things if it would take too long to review everything.

My Project

Workbook_Open:

``
Option Explicit

Private Sub Workbook_Open()

Dim StrTitle As String
Dim StrDateString As String
Dim StrDay As String
Dim StrMonth As String
Dim StrYear As String

StrDay = day(Date)
StrMonth = MonthName(month(Date))
StrYear = year(Date)

StrDateString = StrDay & " " & StrMonth & " " & StrYear
StrTitle = "Weekly Market Recap - " & StrDateString

Sheets("Market Dashboard").Activate
Cells(1, 1).Value = StrTitle

Dim StrFileDestination As String
Dim StrDateNumber As String

StrDateNumber = StrDay & "." & month(Date) & "." & StrYear
StrFileDestination = "S:\Investments\Regular Reports\Market Insight Reports\Weekly Dashboard\Client Facing Dashboard - " & StrDateNumber & ".xlsm"

Dim A As Long
Dim B As Long
Dim C As Long
Dim D As String
Dim E As String
Dim F As String
Dim G As String

A = 1

B = month(Date)
C = year(Date)

B = B - 3
If B <= 0 _
Then
B = B + 12
C = C - 1
End If

D = B & "/" & A & "/" & C

B = B - 3
If B <= 0 _
Then
B = B + 12
C = C - 1
End If

E = B & "/" & A & "/" & C

B = B - 3
If B <= 0 _
Then
B = B + 12
C = C - 1
End If

F = B & "/" & A & "/" & C

B = B - 3
If B <= 0 _
Then
B = B + 12
C = C - 1
End If

G = B & "/" & A & "/" & C

Cells(20, 12) = D
Cells(20, 13) = E
Cells(20, 14) = F
Cells(20, 15) = G

Cells(20, 20) = D
Cells(20, 21) = E

ActiveWorkbook.SaveAs filename:=StrFileDestination

End Sub
`

Option Explicit

Option Explicit

That should be in every VBA module you ever write. No excuses. Go to tools > Options > Require Variable Declaration and it will insert it for you automatically.

This is important because, without that statement. If I write:

dim Cell as range
    cel = range("A1")

Excel treats cel as an entirely different variable. And since you didn't define it, Excel assumes it's a variant (also dangerous). And suddenly you have a rogue variable in your program and another which isn't what you think it is.

Option explicit means if Excel encounters a variable you didn't explicitly declare using dim (or other equivalents) it will refuse to compile until you fix it. Automatically catching all your spelling mistakes (and also forcing you to declare your variable types, another good thing).
Variable Naming

This is a great start:

Dim StrTitle As String
Dim StrDateString As String
Dim StrDay As String
Dim StrMonth As String
Dim StrYear As String

It's clear what type of data should be in these variables and what they represent.

This is not:

Dim A As Long
Dim B As Long
Dim C As Long
Dim D As String
Dim E As String
Dim F As String
Dim G As String

It is a pretty much universal convention in all programming that singular character variables (almost exclusively lowercase) are integers/longs, so:

dim i as long, j as long, k as long

is a perfectly normal coding practice, but:

dim D as string, E as string, F as string

most definitely is not.

Not only is it going to confuse and annoy any programmer reading your code (god help them if you leave your company and they actually have to maintain it) but it's just as likely to confuse you in a week/month/year when you come back to it and have no idea what your variables are or what they're doing.

A good summary of Variable Naming best practices. In general, the name of a variable should tell you, at a minimum, what type (string, long, boolean etc.) of data it holds, and what it is supposed to be.

Dim StrTitle As String
Dim StrDateString As String
Dim StrDay As String

These are ok, but could be easily improved like so:

Dim StrTableHeading As String
 Dim StrTodaysDateString As String
 Dim StrThisDay As String

now if you, or anyone else, sees one of those in the middle of a procedure, they'll know exactly what it is.
Readable Code

B = B - 3 If B <= 0 _
     Then
         B = B + 12
         C = C - 1 End If
 
 D = B & "/" & A & "/" & C
 
 
 B = B - 3 If B <= 0 _
     Then
         B = B + 12
         C = C - 1 End If

I haven't got a clue what this is or what it's supposed to be doing. I just have to blindly trust that it's doing what it's supposed to be. Good variable naming will go a long way here. Just replacing variables with good, descriptive, names gives the following:

lngMonth = lngMonth - 3
If lngMonth <= 0 _
    Then
        lngMonth = lngMonth + 12
        lngYear = lngYear - 1
End If

str3MonthsAgoDate = lngMonth & "/" & lngDay & "/" & lngYear

lngMonth = lngMonth - 3
If lngMonth <= 0 _
    Then
        lngMonth = lngMonth + 12
        lngYear = lngYear - 1
End If

At a glance, it becomes perfectly clear what it's doing. You're creating Date Strings for today, 3 months ago, 6 months ago etc.
Refactoring

In simple language, Refactoring is splitting your Macro into lots and lots of smaller pieces (and sub-pieces etc.) that do little, specific things. E.G. your 5 lines to calculate the string for 3 months ago, could be a User-Created-Function that takes any date String and outputs the string from 3 months previously. (or just look up the DateAdd() Function :p )

Generally, any time you find yourself using copy/paste in your code, it's a good sign that what you just copied should be spun out as it's own sub/function. This is a Good Overview of the concept
Indentation

Another key aspect of readable code. Humans find it much easier to read down the page than across it. If you have loops, nesting, discrete steps etc. It's much clearer if they're all at different levels of indentation. For instance, your Workbook_Open reads much more clearly like this:

```
Private Sub Workbook_Open()

Dim StrTitle As String
Dim StrDateString As String
Dim StrDay As String
Dim StrMonth As String
Dim StrYear As String

Dim StrFileDestination As String
Dim StrDateNumber As String

StrDateNumber = StrDay & "." & month(Date) & "." & StrYear
StrFileDestination = "S:\Investments\Regular Reports\Market Insight Reports\Weekly Dashboard\Client Facing Dashboard - " & StrDateNumber & ".xlsm"

StrDay = day(Date)
StrMonth = MonthName(month(Date))
StrYear = year(Date)

StrDateString = StrDay & " " & StrMonth & " " & StrYear
StrTitle = "Weekly Market Recap - " & StrDateString

Sheets("Market Dashboard").Activate
Cells(1, 1).Value = StrTitle

Dim A As Long

StackExchange Code Review Q#102334, answer score: 7