General function to test for empty/no-value controls

Question

I'm working on a very CRUD-heavy MS Access application, which has over 50 forms for entering and working with data. This, of course, involves lot of testing to see if a textbox, recordset, etc. has a value and then doing something with it.

To prevent mistakes such as testing for IF not rst.BOF and rst.EOF or simply having to look up a test I don't remember, I've created a single function that runs all of my empty/no value tests. Simply pass NOVALUE() an object variable and it uses the Typename() function to select the correct test and return a true/false. No more thinking about tests and no chance of introducing a bug at this point.

The function's purpose is to see if the user has input/selected something or if a recordset has not returned any records. I don't care whether the .RecordSource of a cascading combo/listbox has no rows because the object will also have no value. Also, I'm only using native Access controls, so the function will not test a TreeView or other ActiveX control.

Public Function NoValue(ObjectToTest As Object) As Boolean

    Dim noVal As Boolean

    NoValue = True 

    If ObjectToTest Is Nothing Then
        noVal = True
    Else

        Select Case TypeName(ObjectToTest)

            Case Is = "CheckBox"

                noVal = IsNull(ObjectToTest.Value)

            Case Is = "ComboBox"

                If ObjectToTest.ItemsSelected.Count = 0 Then
                    noVal = True
                Else
                    noVal = False
                End If

            Case Is = "Label"
                If Len(ObjectToTest.Caption) = 0 Then
                    noVal = True
                Else
                    noVal = False
                End If

            Case Is = "Listbox"

                If ObjectToTest.ItemsSelected.Count = 0 Then
                    noVal = True
                Else
                    noVal = False
                End If

            Case Is = "Recordset"    '* ADO
                If ObjectToTest.State = adStateClosed Then
                    noVal = True
                ElseIf ObjectToTest.BOF And ObjectToTest.EOF Then
                    noVal = True
                Else
                    noVal = False
                End If

            Case Is = "Recordset2"    '* DAO
                If ObjectToTest.BOF And ObjectToTest.EOF Then
                    noVal = True
                Else
                    noVal = False
                End If

            Case Is = "TextBox"
                If Len(ObjectToTest.Value & vbNullString) = 0 Then
                    noVal = True
                End If

            Case Else
                ' Only using native access types, not activex, so there shouldn't be anything here
                ' Should I err.Raise ?

        End Select

    End If

    NoValue = noVal

 End Function

Here's the reasoning behind the tests:

• Begin with a test for ObjectToTest Is Nothing to make sure that the object variable has been initialized.
• ADO Recordset: If both .bof and .eof are true, an ADO recordset has no records. I've added the test for closed recordsets because I've occasionally come across them.
• DAO Recordset: Just the standard test for .bof and .eof. There is no way to check for a closed DAO recordset.
• Checkbox: Testing for null, because 0 and -1 (false/true) are values.
• Combobox/Listbox: There are two possible tests that can be performed here, depending on how you want to handle table values that are not in the object's .RowSource.
• Labels: Although these aren't data-bound controls they can have their captions set in code, so I've included them for the sake of consistency -- I really don't want to have to think about testing controls.
• Testing the to see if object's .ControlSource Is NULL will return false if the .ControlSource has a value (i.e. there is a value in the underlying table). A combo/listbox with an invalid value in the underlying table (a value that doesn't appear in the .RowSource) not be NULL, but no item will be selected in the list.
• Testing .ItemsSelected.Count will tell us whether the user has selected an item, regardless of the value in the underlying table. Before the record is saved, the underlying value in the table may be null, but .ItemsSelected.Count will reflect the user's selection. This is what I'm interested in.
• Textboxes: A textbox that contains an empty string will not be null, even though it has no value for my purposes. So, the function tests for both NULL and an empty string. I always use the constant vbNullString instead of "" in my code so there is no chance of confusing " " with "".

Questions:

• Are all of my tests correct? Are there any edge-cases that I have missed?
• My SELECT ... CASE should probably contain a "CASE ELSE" to catch any other object that gets passed to the function. Does it make the most sense to raise an error at this point, or to take some other action?
• Anything else I may have missed or not thought of?

Answer 1

Case Is = "CheckBox"

That syntax is very weird and unusual, surprising even.

This would be more common, and simpler:

Case "Recordset"

TypeName makes a stringly-typed check, which is bug-prone. You say you're not going to use it with any ActiveX controls, but can you be sure no one in the future will ever try to do that? ADODB.Recordset isn't an ActiveX control - neither is DAO.Recordset. So you've already started using it for more than controls.

Consider using the much more robust TypeOf {object} Is {type} approach instead - because of how TypeOf expressions work, you'll have to put the condition in each Case statement for it to work:

Select Case True
    Case TypeOf ObjectToTest Is ADODB.Recordset
        'we have an ADODB.Recordset instance

    Case TypeOf ObjectToTest Is DAO.Recordset
        'we have a DAO.Recordset instance

    '...
End Select

I don't do much MS-Access, but if "form controls" in Access are anything like Excel's, then the Case blocks for these would look like:

    Case TypeOf ObjectToTest Is Access.Label
        '...

That way you can handle this case separately when the time comes:

    Case TypeOf ObjectToTest Is MSForms.Label
        '...

---

HasNoValue isn't instinctive, or desirable. Consider the client code that checks whether a control has a value:

If Not HasNoValue(SomeControl) Then

Avoid double-negatives wherever possible. So, invert the whole logic, so that the client code doesn't have them no matter what:

If HasValue(SomeControl) Then

Or

If Not HasValue(SomeControl) Then

Much cleaner.

---

If ObjectToTest Is Nothing Then
    noVal = True

You're hiding a bug here: if client code passes a null object reference (aka Nothing), you want to blow up right there and then - not pretend you were given a control that "has no value": you're misleading the client code into believing everything is fine & well, when there's a serious problem going on. Best fail fast and hard than make the bug even harder to track down by ignoring invalid state.

This would be my guard clause:

If ObjectToTest Is Nothing Then Err.Raise 91, , "ObjectToTest cannot be a null reference."

Given invalid input, scream - don't push the bug further down the call stack.

Answer 2

no chance of introducing a bug at this point - Oh ye of little faith :) there will always be bugs. Always. But seriously, this is very good for the most part. I only have a few suggestions.

Since your function returns a bool, I would suggest renaming it to HasNoValue(...).

Your function also has too many responsibilities. What happens when you support more controls or types? You'd have to make this function much longer. I'd recommend making each type have its own validation check.

Also, I would host these validation checks in a separate module called Validation.

For each type, you could name its function to something like TypeHasNoValue like CheckboxHasNoValue. When calling the code, you could use the module name in front to fully qualify it like Validation.CheckboxHasNoValue(...)

In module Validation:

Public Function CheckboxHasNoValue(ByRef checkbox As Chekbox) As Boolean
    CheckboxHasNoValue = IsNull(checkbox.Value)
End Function

I know this might seem like overkill, but if you later want to include something like a default (something like an empty item), then you can also check for that in this function whose responsibility is checking a Checkbox for a value.

In your main module, your function becomes something like:

Public Function HasNoValue(ObjectToTest As Object) As Boolean

    Dim noVal As Boolean

    NoValue = True 

    If ObjectToTest Is Nothing Then
        noVal = True
    Else
        Select Case TypeName(ObjectToTest)
            Case Is = "CheckBox"
                noVal = Validation.CheckboxHasNoValue(ObjectToTest)

            Case Is = "ComboBox"
                noVal = Validation.ComboBoxHasNoValue(ObjectToTest)

            Case Is = "Label"
                noVal = Validation.LabelHasNoValue(ObjectToTest)

            Case Is = "Listbox"
                noVal = Validation.ListboxHasNoValue(ObjectToTest)

            Case Is = "Recordset"    '* ADO
                noVal = Validation.RecordsetHasNoValue(ObjectToTest)

            Case Is = "Recordset2"    '* DAO
                noVal = Validation.Recordset2HasNoValue(ObjectToTest)

            Case Is = "TextBox"
                noVal = Validation.TextBoxHasNoValue(ObjectToTest)

            Case Else
                ' Only using native access types, not activex, so there shouldn't be anything here
                ' Should I err.Raise ?

        End Select

    End If

    NoValue = noVal

 End Function

As for your question about Case Else, yes I think you should have one. But about the behavior, that really depends on your application. Do you have error handling? Do you have a log? Do you just want to silently fail because maybe it's not critical? It really depends. But, yes, you should have one for completion sake.

Answer 3

I was finding that when I inspected a control on a form for a value an error was being raised if the form had no recordset but the control had a controlsource.

Taking into consideration what Mathieu Guindon wrote in his answer. I used the example given in the question to create the two functions below that I think may be of use to people reading this question!

    Public Function HasValue(ControlObject As Control, Optional ControlsValue As Variant) As Boolean

    ' IN    ControlObject - The object to inspect to see whether it has a value
    ' OUT   TheValue     - A ByRef parameter that allows this to return the value of the object
    '                      if it is determined to have one!

    Dim HasVal As Variant

    On Error GoTo Err1

    ' The code below should always set HasVal to either be True or False
    HasVal = Null

    ' Return NULL if an object has 'no value'.
    ' (NULL chosen as this is a database environment where fields are NULL if no data is stored by them.)
    ControlsValue = Null

    If ControlObject Is Nothing Then

        HasVal = False
        ControlsValue = Null

    ElseIf TypeOf ControlObject Is Label Then

        If Len(ControlObject.Caption) = 0 Then
            HasVal = False
            ControlsValue = Null
        Else
            HasVal = True
            ControlsValue = ControlObject.Caption
        End If

    Else

        ' Note that error 2424 is raised if the Value proeprty of the following controls is
        ' referred to in code when
        ' 1. The control's 'Control Source' property is set to a field in the forms recordsource.
        ' and
        ' 2. the Form.RowSource property is empty.
        '
        ' Note a control's control source property can be set to text that does not refer to the recordset
        ' in which case no error is raised!  eg =MyFunction(1,2,3)
        ' So no test can be added here to predict when the above error (error 2424) is raised.
        '

        If TypeOf ControlObject Is TextBox _
            Or TypeOf ControlObject Is ListBox _
            Or TypeOf ControlObject Is ComboBox _
            Or TypeOf ControlObject Is CheckBox _
        Then

            On Error GoTo ErrNoValueProperty

                HasVal = Not IsNull(ControlObject.Value)
                ' Note that the above treats an empty string as being a value.
                'If ControlObject.Value = "" Then
                '    HasVal = False
                'End If

                ControlsValue = ControlObject.Value

            On Error GoTo Err1

        Else

            Err.Raise cnstUserDefinedErrorLowest + 1, "", "Unknown ControlObject."

        End If

    End If

TidyUpAfterNoValueProperty:

    HasValue = CBool(HasVal)

Exit Function

ErrNoValueProperty:

    If Err.Number = 2424 Then
        '
        ' 2424 = "The expression you entered has a field, control, or property name
        '         that This Database can't find."
        '
        ' See comments in code above for explaination!
        '
        HasVal = False
        GoTo TidyUpAfterNoValueProperty

    Else
        GoTo Err1

    End If

Err1:
    MyErrDescrition = BuildErrorDescription("HasValue", cnstModuleName)
    Err.Raise Err.Number, Err.Source, MyErrDescrition
    Exit Function
    Resume

End Function

Public Function NzForObjects(ControlObject As Object, ValueIfUnEntered As Variant) As Variant

    Dim ControlsValue As Variant

    On Error GoTo Err1

    If HasValue(ControlObject:=ControlObject, ControlsValue:=ControlsValue) Then

        NzForObjects = ControlsValue

    Else

        NzForObjects = ValueIfUnEntered

    End If


Exit Function
Err1:
    MyErrDescrition = BuildErrorDescription("NzForObjects", cnstModuleName)
    Err.Raise Err.Number, Err.Source, MyErrDescrition
    Exit Function
    Resume

End Function

Answer 4

A few minor things that weren't explicitly picked up on in the other answers;

If ObjectToTest.ItemsSelected.Count = 0 Then
    noVal = True
Else
    noVal = False
End If

My personal preference in Boolean assigning blocks like these is a one liner:

noVal = ObjectToTest.ItemsSelected.Count = 0

Which I find scans better and avoids the next problem (although I'm aware some people are squeamish about this sort of thing and prefer to be explicitly explicit)

---

If you do use the If block at least be consistent:

Case Is = "TextBox"
    If Len(ObjectToTest.Value & vbNullString) = 0 Then
        noVal = True
    End If

Case Else

Here you have no Else statement and instead rely on the default initial value of noVal as False, which means if your logic changes and you end up writing to the variable earlier in the procedure, this check will break. The one liner helps to avoid such inconsistencies.

---

While we're here, I'd also like to make a brief point about comments; comments are there to explain why you have taken certain decisions in your code. Your check for textboxes is, to me at least, non-obvious on first glance - so a comment to explain why would be helpful:

noVal = Len(ObjectToTest.Value & vbNullString) = 0
'Append vbNullString so the function tests for both NULL and an empty string

In fact for that matter, any time you've explained why your code does something in your question body, you should ideally have a comment instead (or your code should be self documenting with helpful variable names and abstraction, which for the most part it is)

---

Case Else
    ' Only using native access types, not activex, so there shouldn't be anything here
    ' Should I err.Raise ?

Well that depends; as @MathieuGuindon correctly points out, the last thing you want to do is to hide invalid input. The best way to think about this is to imagine what happens if you do supply an ObjectToTest which you think should have a check but doesn't. Would you want a default return value (or a default check) for unimplemented cases? Maybe - but unlikely. Would you want to be made aware that the check you are trying to make is not implemented? Probably.

So yes, I would recommend some sort of notification here - be it an error or a warning message or Stop. Personally I'd go for the error because it is explicit and logabble and even supressable, something along the lines of

Err.Raise 5, , "No check implemented for objects of type " & Typename(ObjectToTest)

Where the 5 corresponds to an invalid argument (see trappable errors). Use a named Const if you want, I'm lazy and don't bother if it's just in one place in the procedure.