Retrieves Data From Various Excel Sheets Online, sorts, edits, and Analyzes said Data

This is a routine I wrote to streamline analysis I need to do that originally took 1.5 hours and now takes about 4 minutes total run time.

I'm fairly new to Excel VBA so I welcome all criticism and advice!

```
Option Explicit

#If VBA7 And Win64 Then
Private Declare PtrSafe Function URLDownloadToFile Lib "urlmon" _
Alias "URLDownloadToFileA" ( _
ByVal pCaller As LongPtr, _
ByVal szURL As String, _
ByVal szFileName As String, _
ByVal dwReserved As LongPtr, _
ByVal lpfnCB As LongPtr _
) As Long
Private Declare PtrSafe Function DeleteUrlCacheEntry Lib "Wininet.dll" _
Alias "DeleteUrlCacheEntryA" ( _
ByVal lpszUrlName As String _
) As Long
#Else
Private Declare Function URLDownloadToFile Lib "urlmon" _
Alias "URLDownloadToFileA" ( _
ByVal pCaller As Long, _
ByVal szURL As String, _
ByVal szFileName As String, _
ByVal dwReserved As Long, _
ByVal lpfnCB As Long _
) As Long
Private Declare Function DeleteUrlCacheEntry Lib "Wininet.dll" _
Alias "DeleteUrlCacheEntryA" ( _
ByVal lpszUrlName As String _
) As Long
#End If

Public Const ERROR_SUCCESS As Long = 0
Public Const BINDF_GETNEWESTVERSION As Long = &H10
Public Const INTERNET_FLAG_RELOAD As Long = &H80000000
Public Const csURL As String = "http://www.cmegroup.com/CmeWS/exp/voiProductDetailsViewExport.ctl?media=xls&tradeDate=×d×&reportType=P&productId=×p×"

Sub Pull_CME_Data()

Dim getBook As String, OiProductCodesArray() As String
Dim cmeDataBook As Workbook
Dim data As Worksheet, controlPanel As Worksheet, lo As Worksheet, wa As Worksheet, sevenA As Worksheet, oh As Worksheet, at As Worksheet, ob As Worksheet, ra As Worksheet, cvr As Worksheet, ln As Worksheet, wtioi As Worksheet, rboboi As Worksheet, ethanoloi As Worksheet, heatoi As Worksheet, sevenaoi As Worksheet, waoi As Worksheet, ngoi As Worksheet, atoi As Worksheet, raoi As Worksheet, volumeSheet As Wo

A wise programmer once told me


What a fascinating solution! It seems that for every smart decision you made, you also threw in a poor decision or two.

This is how your code makes me feel. You clearly know a fair amount of technical details about implementing code that does stuff, but very little about implementing code that is actually useful. Code is only useful if you (and whoever else ever has to read/maintain/extend it) can easily understand what it is doing and why.

There's a lot here which I may get to later, but right now I'm going to focus on your naming, because it's really hard to understand your code when I have to stop every line to figure out what the hell each variable is trying to represent.

Names should be Descriptive, Unambiguous and Concise. In that order.

I should be able to read a variable name and instantly know what the variable represents, its' scope and a pretty good idea of its' data type.

cmeDataBook is a good variable name. It is a workbook, that contains the cme data. It is a workbook object, and its' in camelCase so its' scope is local to this procedure. And I can tell all that by reading it just once.

oiProductCodesArrayCounter is another good variable name. It's a counter (probably an index counter) for the oiProductCodesArray. Awesome.

This: WshtNameCrnt is not a good name. I think it's trying to say currentWorksheetName but I had to stop and think for a good 5-10 seconds. And I'm still not entirely sure that's right. Screen real estate is cheap. Cognitive processing and understanding is not.

On that subject: I see getBook and I think "That's a method to get a workbook object? Maybe a boolean to denote whether a workbook should be retrieved?". Wait, what do you mean that's the name of the active workbook? Why on earth wouldn't you call it activeWorkbookName!

Continuing in the same vein: If something is a Worksheet object then that should be obvious from its' name. Rather than data, which could be meant to represent literally anything , how about wsData? just 2 characters makes it so much clearer what's going on here. wsLo, wsWa, wsOh, wsAt, the abbreviations are still painful to keep track of, but at least when they pop up in the code later, I'm going to know roughly what they are.

Your naming conventions are all over the place.

You should follow standard VBA naming conventions wherever possible. It allows other people to seamlessly absorb extra metaData about your code, and it allows you to do the same with the vast majority of professional VBA code.

Namely:

camelCase means a variable is local to a procedure.
Dim localVariable

PascalCase means a variable is global to a Module or the entire project.
Private ModuleVariable, Public GlobalVariable
It is also used for method names:
Public Sub DoThisThing()

SHOUTY_SNAKE_CASE is used for constants
Public/Private Const CONSTANT_VALUE

Method Names generally use _ to denote event triggers. E.G.
Private Sub Workbook_Open()
So don't use them in your other Method Names.

most of your code follows these conventions, but there are enough variables that don't to render the whole thing worthless. Examples:

Public Const csURL As String
, OiProductCodesArray() As String
Dim ClearWshtsArray As Variant
Dim WshtNameCrnt As Variant
Private Function seperateData

Miscellaneous things that jump out at me:

Dim WshtNameCrnt As Variant

If this thing is always going to be used to hold text, then why not dim it as a string?

Call is deprecated. These 2 statements are functionaly identical:

Call seperateData(cmeDataBook, data, cvr, "cvr", 5)
seperateData cmeDataBook, data, cvr, "cvr", 5

Integer is also deprecated. The compiler will silently convert all Integers to Longs, so just use Long.

Explicitly scope everything. This includes method arguments

Sub GetTotalVolumeData(volumeSheet As Worksheet, controlSheet As Worksheet)

Are those meant to be passed ByRef or ByVal?, because right now, they're implicitly being passed ByRef.

Codenames

Codenames are big and clever. Every worksheet and workbook has a "name" that the user can see and change.

Set data = cmeDataBook.Sheets("Data")

is referencing a sheet name.

A Codename on the other hand is a secret name that can only be set/changed in the IDE.

the name in brackets is the "name". The name not in brackets is the "codename". It is set in the properties window.

If you give a sheet a codename (E.G. wsData) then the user can change the name as much as they like, all you have to do is use

wsData.Name

in your code and it will keep running.

It also means you can just delete this entire section of code:

```
getBook = ActiveWorkbook.Name
Set cmeDataBook = Workbooks(getBook)
Set data = cmeDataBook.Sheets("Data")
Set controlPanel = cmeDataBook.Sheets("Control Panel")
Set lo = cmeDataBook.Sheets("Lo")
Set wa = cmeDataBook.Sheets("Wa")
Set sevenA = cmeDataBook.Sheets("7A")
Set o

StackExchange Code Review Q#120962, answer score: 4