As Matt has provided an alternative solution I'll focus more on improving your current approach (well that was covered too but there are some extra things)
' Check if a name is valid: it may be "declared" in Excel using LET(). Public Function NameIsValid(name As String) As Boolean
Using Let is an implementation detail (as Matt demonstrated with an alternative implementation), and not something the caller really needs to worry about. Instead focus on input/output. You could use a RubberDuck description annotation here which means, once synced, this comment magically turns into a description in the VBA object explorer.
'@Description("Returns True if candidate is a valid name for an Excel named reference, False if not or the check fails")
' Invalidate names that are empty or too long. If name = Empty Or VBA.Len(name) > 255 Then
This comment is unnecessary as we can see what happens on the next line - at least qualify too long as
'Excel names have a maximum length
to explain why you perform the check, or better yet get rid of 255 and use a named constant:
Const MAX_NAME_LENGTH As Long = 255
If name = vbNullString Or VBA.Len(name) > MAX_NAME_LENGTH Then
NameIsValid = False
No comment required. You want to eliminate comments that explain what the code does where possible because if your code changes the comment becomes out of sync.
While we're here, NameIsValid = False is actually unnecessary because False is the default value of a boolean function. I would prefer to write Exit Function here, which has the side effect of allowing you to reduce the nesting of your code:
If name = Empty Or VBA.Len(name) > 255 Then Exit Function
If ( _
name = "C" Or name = "c" Or _
name = "R" Or name = "r" _
) Then Exit Function
If name <> Application.WorksheetFunction.Clean(VBA.Trim(name)) Then Exit Function
If ( _
VBA.InStr(1, name, "(") Or _
VBA.InStr(1, name, ",") Or _
VBA.InStr(1, name, ";") Or _
VBA.InStr(1, name, ")") _
) Then Exit Function
' Get the result of formulaically declaring a name with LET() in Excel.
Dim eval As Variant
eval = Application.Evaluate("= LET(" & name & ", 0, 0)")
' Check if the declaration erred due to invalid nomenclature.
NameIsValid = IsError(eval)
' Invalidate names with external whitespace (or double spaces internally), ' which are invalid in names and yet could mesh syntactically with ' formulaic calls to LET() in Excel.
This is a great comment, explains a non-obvious bit of code.
name = "C" Or name = "c" Or _ name = "R" Or name = "r"
VBA.InStr(1, name, "(") Or _ VBA.InStr(1, name, ",") Or _ VBA.InStr(1, name, ";") Or _ VBA.InStr(1, name, ")") _
can both be syntactically simplified with the Like operator:
If name Like "[cCrR]" Then Exit Function 'catches single character reserved names
If name Like "*[(,;)]*" Then Exit Function 'catches at least one occurence of invalid character
although I'm not sure what that would do for speed. The InStr functions are definitely your slow point (if not the evaluate call); not only does checking require iterating over every character until the correct one is found, VBA does not short circuit operators so always performs all 4 checks even if the first one fails. Split onto 4 lines if performance is key. In any language worth its salt the Like operator should be fast and compiled - but as Matt says VBA's string operators and functions are notoriously slow.
ElseIf name <> Application.WorksheetFunction.Clean(VBA.Trim(name)) Then
NameIsValid = False
' Invalidate names with injection characters, which are invalid in names
' and also disrupt formulaic calls to LET() in Excel.
ElseIf ( _
VBA.InStr(1, name, "(") Or _
VBA.InStr(1, name, ",") Or _
VBA.InStr(1, name, ";") Or _
VBA.InStr(1, name, ")") _
) Then
NameIsValid = False
' If we pass the above checks, we can safely splice the name into a
' formulaic declaration with LET() in Excel.
Else
' Get the result of formulaically declaring a name with LET() in Excel.
Dim eval As Variant
eval = Application.Evaluate("= LET(" & name & ", 0, 0)")
' Check if the declaration erred due to invalid nomenclature.
If IsError(eval) Then
NameIsValid = False
Else
NameIsValid = True
End If
End If
End Function
Questions
- Am I missing any subtle (or obvious) edge cases in my design? It is risky and rarely best practice to
Evaluate()a literalStringas code. Furthermore, I am wary of assuming that I have innovated a (somewhat trivial) solution that escaped the meticulous author of excel-names.
I wouldn't be too wary about a novel solution; LET was introduced in the last couple of years, that workbook was last updated in 2017.
- Should I distinguish granularly between error types, and only invalidate the
namefor specific reasons?
Yes IMO you should only surpress errors you expect, other errors you rethrow to the user (in case this is consumed by VBA) or return as an error if this is a UDF:
'@Description("Returns True if candidate is a valid name for an Excel named reference, False if not, and an error if the check fails")
Public Function NameIsValid(name As String) As Variant
'...
If IsError(eval) Then
' Granularly distinguish between specific errors.
If ( _
eval = CVErr(xlErrName) Or _
eval = CVErr(xlErrValue) _
) Then
NameIsValid = False
Else
NameIsValid = eval 'return the unknown error
End If
Else
NameIsValid = True
End If
End Function