excelvba

Best practice for checking if an cell entry will generate an error


Situation:

I am using the content of cell "D2" name the work sheet. The code below I just edited to catch various various problem entries, post a message, and supply a value that works and is obvious that needs to be replaced.

Code:

Private Sub Worksheet_Change(ByVal Target As Range)
    
    Dim BadData As Boolean
    
    If Intersect(Target, ThisWorkbook.Sheets(2).Range("D2")) Is Nothing Then
        Exit Sub
    End If
    
   BadData = False
        
    If Len(Target.Value) > 31 Or IsEmpty(Target.Value) Then
        BadData = True
    ElseIf InStr(Target.Value, "[") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, "]") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, "*") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, "?") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, "\") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, "/") <> 0 Then
        BadData = True
    ElseIf InStr(Target.Value, ":") <> 0 Then
        BadData = True
    End If
    
    If BadData Then
        MsgBox "You have entered an unacceptable ID value." & vbCrLf & _
                vbCrLf & _
                "The Site No entry must:" & vbCrLf & _
                vbCrLf & _
                "1)  Be a value less than 31 characters long" & vbCrLf & _
                "2)  Not contain the following characters:  /, \, :, ?, *, [, ]" & vbCrLf & _
                "3)  Not be a blank value" & vbCrLf & _
                "4)  Be compatible with excel sheet name restrictions"
                
        Target.Value = "Enter Site ID"
    End If
    
    ThisWorkbook.Sheets(2).Name = Target.Value

End Sub

While the code above appears to work, I am sure there a multiple other ways of achieving the same end result. The multiple IF statement resulting in the same value bothered me a little.

I originally though to put all the individual InStr check into one long OR statement with the Len and IS Empty check, but choose not to as I figured that would be a pain for someone else to come in an try and figured out the code. I realised that there may be some benefit of ELSEIF preventing subsequent checks from being made if a TRUE check resulted.

Question

I was wondering if there was a better way to do this?


Solution

  • You can put it in a function then use that? This way you can maintain (add, remove conditions) the function.

    '~~> UNTESTED
    Private Function IsBadData(CellValue As Variant) As Boolean
        Dim BadString As String
        Dim BadData As Boolean
        Dim MyAr
        Dim i As Long
        
        BadString = "[,],*,?,\,/,:"
        
        If Len(Trim(CellValue)) = 0 or Len(Trim(CellValue)) > 31 Then
            BadData = True
        Else
            MyAr = Split(BadString, ",")
        
            For i = LBound(MyAr) To UBound(MyAr)
                If InStr(1, CellValue, MyAr(i)) Then
                    BadData = True
                    Exit For
                End If
            Next i
        End If
        
        IsBadData = BadData
    End Function
    

    Usage:

    If IsBadData(Target.Value) Then
        MsgBox "Blah Blah"
    End If