Strong preference should be given to writing code that is self-documenting. A common approach is to write a lot of comments, but the problem with comments are twofold:
- It actually can harm the readability because it adds to the overall length of code to read and understand.
- It must be correct or it can end up wasting time. This is illustrated well with a silly gif:
If you feel that a procedure must be commented because it’s doing complex, the next question you need to ask yourself is:
Is the procedure doing too much?
To illustrate why it can be a problem, consider the following procedure:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 |
'Use this only to send emails to clients Private Sub SendEmail() Dim objOutlook As Object 'Outlook.Application Dim objEmail As Object 'Outlook.MailItem Dim fol As Object 'Scripting.Folder Dim fil As Object 'Scripting.File 'Let's try to see if Outlook is open, otherwise make a new instance On Error Resume Next Set objOutlook = GetObject(, "Outlook.Application") If Err.Number Then Set objOutlook = Nothing End If On Error GoTo 0 If objOutlook Is Nothing Then 'We didn't get it, so we'll need to new it up Set objOutlook = CreateObject("Outlook.Application") End If Set objEmail = objOutlook.CreateItem(0) '0 means email 'Now we have the email, let's get the clients' name objEmail.To = DLookup("Email", "tblEmployee", "EmployeeID = " & TempVars!EmployeeID) objEmail.Subject = "Updated documents" objEmail.Body = "We've made updates to the documents. Please login or see attached." 'We need to get attachments, if any Set fol = TempVars!FolderName For Each fil In fol.Files objEmail.Attachments.Add fil.Path Next objEmail.Send End Sub |
If you read the entire procedure and understood what it does, mad props. However, this would read much easily:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 |
Public Sub SendEmail( _ EmployeeIDToBeEmailed As Long, _ AttachmentFolderPath As String _ ) Dim App As Object 'Outlook.Application Dim Email As Object 'Outlook.MailItem #If LateBind Then Const olMailItem As Long = 0 #End If Set App = OutlookApp() Set Email = App.CreateItem(olMailItem) AddAttachments Email.Attachments, AttachmentFolderPath With Email .To = DLookup("Email", "tblEmployee", "EmployeeID = " & EmployeeIDToBeEmailed) .Subject = "Updated documents" .Body = "We've made updates to the documents. Please login or see attached." .Send End With End Sub Private Sub AddAttachments( _ RefAttachments As Object, _ AttachmentFolderPath As String _ ) Dim fol As Object 'Scripting.Folder Dim fil As Object 'Scripting.File Set fol = AttachmentFolderPath For Each fil In fol.Files RefAttachments.Add fil.path Next End Sub |
Though the 2nd version is 3 more lines, there are much less comments for following reasons:
- Instead of having to explain what the code does, the arguments the procedure requires makes it clear that it expects an employee to send an email to which is embedded in the parameter name EmployeeIDToBeEmailed . In first version, we had a TempVars!EmployeeID, which would lead to surprises if it doesn’t have the value we expect.
- Instead of having one large procedure that does 3 things:
- Get an instance of Outlook.Application
- Create an email
- Add attachments
- Instead of having a comment to each magical numbers, we define a constant that self-describes, in this case, the olMailItem, which is also already defined in Outlook object library but even if it was our own constant, it would still have a descriptive name and thus read much easier than having a comment affixed.
Legitimate reasons to comment
However, this does not mean comments do not have their place. The main reason why you should comment your code is to explain the why, not the how of the code. A good example would be something like this:
|
1 2 3 4 5 6 7 |
Private Sub SendFTP(UserName As String, Password As String) 'Due to a bug in FTP library, we must first open a channel then authenticate MyFTP.Open() MyFTP.Authenticate(UserName, Password) MyFTP.Open() ... End Sub |
Another reason is to provide context where it is not possible. You saw an example of this earlier with this line:
|
1 |
Dim App As Object 'Outlook.Application |
Because the code was late-bound, it provides information that we cannot embed in the code anyway.
