Access JumpStart 2.0 | Blog

A Rapid Development Framework for Microsoft Access

Given the following code, I am still refactoring it:

Private Function FieldChanged(Field As Access.Control) As Boolean
    Dim retVal As Boolean
    If OnlyOneFieldIsNull(Field.OldValue, Field.Value) Then
        retVal = True
    ElseIf IsNull(Field.OldValue) And IsNull(Field.Value) Then
        retVal = False
    Else
        If Field.OldValue <> Field.Value Then
            retVal = True
        End If
    End If
    FieldChanged = retVal
End Function

Private Function OnlyOneFieldIsNull(OldValue As Variant, NewValue As Variant) As Boolean
    Dim retVal As Boolean
    retVal = (IsNull(OldValue) = True And IsNull(NewValue) = False) Or (IsNull(OldValue) = False And IsNull(NewValue) = True)
    OnlyOneFieldIsNull = retVal
End Function

I’ve pulled out the FieldChanged function from the BeforeUpdate event already, see the TDD – 036 article for that. I still have way to many If blocks in that first function, and I don’t really like the readability of the second function. It is clear to me what it is doing by the name though: OnlyOneFieldIsNull, takes two values and returns a boolean.

All the function is really doing is checking both conditions where 1 is null and 2 is not null, or 1 is not null and 2 is null. Maybe just using Not instead of a true false check will be more readable:

Private Function OnlyOneFieldIsNull(OldValue As Variant, NewValue As Variant) As Boolean
    Dim retVal As Boolean
    retVal = (IsNull(OldValue) And Not IsNull(NewValue)) Or (Not IsNull(OldValue) And IsNull(NewValue))
    OnlyOneFieldIsNull = retVal
End Function

It’s a little shorter. What if I add one more line to get rid of the parentheses and Or condition?

Private Function OnlyOneFieldIsNull(OldValue As Variant, NewValue As Variant) As Boolean
    Dim retVal As Boolean
    retVal = IsNull(OldValue) And Not IsNull(NewValue)
    If Not retVal Then retVal = Not IsNull(OldValue) And IsNull(NewValue)
    OnlyOneFieldIsNull = retVal
End Function

Actually, the above introduces possible logic errors because I’m trying to be clever. I think I’ll just create new variable names to document the conditions better.

Private Function OnlyOneFieldIsNull(OldValue As Variant, NewValue As Variant) As Boolean
    Dim retVal As Boolean, OldIsNullNewIsNot As Boolean, OldIsNotNullNewIs As Boolean
    OldIsNullNewIsNot = IsNull(OldValue) And Not IsNull(NewValue)
    OldIsNotNullNewIs = Not IsNull(OldValue) And IsNull(NewValue)
    retVal = OldIsNullNewIsNot Or OldIsNotNullNewIs
    OnlyOneFieldIsNull = retVal
End Function

Eh, I suppose I could make each of the variables functions which would then make this more readable as well. Ah, ok, I’ll do it.

Private Function OnlyOneFieldIsNull(OldValue As Variant, NewValue As Variant) As Boolean
    OnlyOneFieldIsNull = OldIsNullNewIsNot Or OldIsNotNullNewIs
End Function

Private Function OldIsNullNewIsNot(OldValue As Variant, NewValue As Variant) As Boolean
    OldIsNullNewIsNot = IsNull(OldValue) And Not IsNull(NewValue)
End Function

Private Function OldIsNotNullNewIs(OldValue As Variant, NewValue As Variant) As Boolean
    OldIsNotNullNewIs = Not IsNull(OldValue) And IsNull(NewValue)
End Function

Ok, now it’s obvious I think what’s going on. I also removed the retVal variable I usually use because I don’t think I’ll need to revisit these functions and if I rename them I still just need to rename one line. So far so good. Now I shall turn my attention to the bigger function:

Private Function FieldChanged(Field As Access.Control) As Boolean
    Dim retVal As Boolean
    If OnlyOneFieldIsNull(Field.OldValue, Field.Value) Then
        retVal = True
    ElseIf IsNull(Field.OldValue) And IsNull(Field.Value) Then
        retVal = False
    Else
        If Field.OldValue <> Field.Value Then
            retVal = True
        End If
    End If
    FieldChanged = retVal
End Function

So after the function to check if one field is null I check if both fields are null. That’s taking care of all the edge case null conditions. I think I can boil this function down to 2 lines. And edge case Null check and a non-null check. I will continue refactoring to my satisfaction next time.