End of day receipt emailing process

The following functions/routines are an automation of an end of day receipt emailing process that formerly would take up to an hour, but now takes less than a minute.

Things to note: I changed the name of some of a variable or two in order to protect the identity of the company that this is being used for. THe vast majority of the variables/objects are uneffected, but several have had to be changed. I am also aware of the ability to change worksheet code names so that they can be utilized like Global Variables, but I am hesistant to change code names for sheets in this specific workbook because there are a lot of other macros that have been written by someone else and i'm not sure how they manipulate the existing worksheet objects. I am confident these worksheet names will never change.

I am open to any and all criticism, especially on how to do things more efficiently and cleanly. I am still fairly new to VBA and i'm looking to write like a professional. I would love to utilize arrays more often, but i'm not quite sure how to search through their contents or how to organize them well. I've only used them to store product codes and things like that to iterate through an array with an index counter. I'd also like some pointers on error handling!

Clarifying the Processes: This code runs off of a main sheet that lists business conducted for the day by client, ReportsByFirmSheet, and generates receipt emails for each client that did business with the company utilizing the sheets. These emails have a general static body that does not change (the to field changes dependent upon the client) and have PDF receipts attached (clients may have multiple receipts throughout the day). ControlPanelSheet is merely the main driving sheet of the workbook where most of the macro execution abilities are stored for client side use. TradesMasterSheet is a big running sheet of raw business data (RepotsByFirmSheet is a less detailed summation of those business transacti

Public Const EMAIL_BODY As String = "Hello," & "" & "Please find today's business receipt's attached.  Thank you." & "" & "Best Regards," & ""

Is there a reason you're using
instead of vbLf or similar?

Declaring variables on the same line like this

Dim firmAlreadyRun As Boolean, isTraderSeparate As Boolean, firmNeedExcelConfirm As Boolean, productSeparatedEmails As Boolean

Is, in my opinion, bad practice. It's great that you've given them all a type, but why not give each variable its own line - it's free and increases readability.

firmName1

It's usually an indication that your variables aren't named descriptively enough when you include a number in them. This could mean it should be firstFirm or currentFirm or nextFirm - however you want to do it so that it's easier to follow.

On the same note, I don't see many variables with names that aren't descriptive enough - so great job on most of your naming!

If firmAlreadyRun = True Then

Doing something like If method = True then is redundant, you can just say If method then.

In the same vein something like

If IsEmpty(contactsMaster.Cells(firmFinder.Row, 6)) = False

could probably just be If Not IsEmpty() Then

Call BuildAddressMailItem(outMail, firmEmail, FormattedReportDate)

You don't need to Call subs, it's obsolete. Instead just use Sub argument, argument

Up top -

Dim FormattedReportDate As String
Dim ReportsByFirmSheet As Worksheet, ControlPanelSheet As Worksheet, TradesMasterSheet As Worksheet

It's usually better to Private them rather than Dim just for clarity.

At the same time - you are mostly rocking Standard VBA naming conventions with pascalCase, CamelCase and SHOUTY_SNAKE_CASE in the correct places.

Private Sub GetExcelConfirms

This one seems like arrow code and could probably be replaced with a SELECT CASE.

In GetPDFConfirms you're using

If currentFirmName <> firmName1 And currentFirmName <> firmName2 And currentFirmName <> firmName3 Then GoTo skipToNext

Seems like you might be better off moving that outside of the loop instead of needing to use the label:.

StackExchange Code Review Q#126656, answer score: 2