# Adding or replacing a sheet in an Excel file

I have the following function and want to simplify the code to make it more readable by eliminating redundant lines:

Function AddSheet(shtName As String, Optional readOnlyFlag As Boolean = True) As Boolean  'Add new worksheet to current workbook at the end

Exit Function
If IsSheet(shtName) Then DeleteSheet (shtName)
With ThisWorkbook:
End With
End If

End Function


Since the first condition (If readOnlyFlag And IsSheet(shtName)) will only assign a return value of False to the function (which should already be the default, as I understand) and then exits the function, would it be acceptable to leave out that aspect of the logic and just have the following:

    If Not readOnlyFlag Then
If IsSheet(shtName) Then DeleteSheet (shtName)
With ThisWorkbook:
End With
End If


In other words, is it best practice to be explicit like the first code block, by stating that the function returns False if that occurs, or to avoid what might be considered redundancy, by using the simplified second version?

VBA does a lot of implicit things. And it can get confusing, and it can cause surprising bugs.

Your function is implicitly Public. Is it meant to be called from another module? Or is it only called from within the same module it's in? Being explicit about everything makes the intent of the code much easier to understand.

The parameters are implicitly passed ByRef, and yet, their values aren't reassigned in the body of the function (which is good!), so they could, and should, be passed ByVal. It's really a shame that ByRef is the implicit default in VB6/VBA.

The comment that's trailing after the function's signature, should be either immediately above, or immediately below the signature - avoid horizontal scrolling, and don't hide information from maintainers.

The comment could be clearer about why a Boolean return value is returned.

Private Function AddSheet(ByVal shtName As String, Optional ByVal readOnlyFlag As Boolean = True) As Boolean
'Appends a new worksheet to the active workbook. Returns True if sheet is successfully added.


It's not clear what readOnlyFlag does, even when reading the code. It probably could use a better name and/or a comment. It seems it determines whether the sheet should be replaced if it already exists?

The IsSheet function looks like it should be called IsExistingSheetName.

The instructions separator (colon) after With ThisWorkbook: serves no purpose and should be omitted; the Exit Function is redundant, too, since the code branches out of that block anyway.

There's an execution path where you're verifying whether the sheet name exists twice - I'd factor that check out of the If block.

As for the return value - I'd go even more explicit than that:

Dim result As Boolean

Dim isExisting As Boolean
isExisting = IsExistingSheetName(shtName)

result = False
If isExisting Then DeleteSheet (shtName)
With ThisWorkbook
End With
result = True
End If



Now, how helpful is that? How does the calling code access that new worksheet now? With Application.ActiveSheet? That's a dangerous assumption to make!

How about returning the Worksheet object instead? The function would return Nothing if no worksheet is added, and an object reference when it is.

Dim result As Worksheet

Dim isExisting As Boolean
isExisting = IsExistingSheetName(shtName)

'Set result = Nothing '' todo: remove this useless branch
If isExisting Then DeleteSheet shtName
With ThisWorkbook
If Not result Is Nothing Then result.Name = shtName
End With
End If


Notice something? The code will blow up if .Add fails, because the function will return Nothing and then .Name cannot be set: you need to handle this.
• Thank you for the extensive response. That's exactly what I wondered. I definitely need to be more careful with ByRef/ByVal statements. I also agree the default is out of whack. That makes me wonder, since objects are passed ByRef, if I pass ThisWorkbook to a function by reference, then in the function, reassign its value, what would happen there? – teepee Mar 3 '16 at 23:15
• Wouldn't ThisWorkbook be implicit to the module? – Raystafarian Mar 4 '16 at 17:10