HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

A global Enum for comparison operators

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
globalenumcomparisonforoperators

Problem

This approach was born out of wanting to pass a logical expression (E.G. ">=10000") as a parameter to a function.

So, I built an Enum and a function to use it to evaluate logical expressions.

Is this a good, maintainable, scalable approach?

Public Enum comparisonOperator
    '/ Used [[ ]] because the outer set escape the inner brackets for the Enum list, which will then escape the operator when chosen as a value. E.G. they will appear as "Operator = [=]] = 4
    [[>]] = 5
End Enum

Public Function ComparisonIsTrue(ByVal sourceValue As Variant, ByVal operator As comparisonOperator, ByVal comparisonValue As Variant) As Boolean

    Dim isTrue As Boolean

    Select Case operator

        Case Is = 1 '/ =
        isTrue = (sourceValue >= comparisonValue)

        Case Is = 5 '/ >
        isTrue = (sourceValue > comparisonValue)

        Case Else
        '/ Error Handling
        Stop

    End Select

    ComparisonIsTrue = isTrue

End Function

Solution

Public Enum comparisonOperator
    '/ Used [[ ]] because the outer set escape the inner brackets for the Enum list, which will then escape the operator when chosen as a value. E.G. they will appear as "Operator = [=]] = 4
    [[>]] = 5
End Enum


Why not just name them and comply with the language's rules for user identifiers? Also, Enum constants should be enumerated: let the compiler assign the underlying values:

Public Enum ComparisonOperator
    EqualTo = 0
    LessThan 
    LessThanOrEqualTo 
    GreaterThanOrEqualTo 
    GreaterThan 
End Enum


There should be a sensible default (0) value; I think I'd make EqualTo that default.

The operators in Case Is = ... are redundant, but the magic numbers in the Case expressions should be the enum named constants. The underlying values should be abstracted away behind the enum - the underlying values should be meaningless.

Consider:

Case ComparisonOperator.LessThan
    isTrue = sourceValue = comparisonValue

    Case ComparisonOperator.GreaterThan
    isTrue = sourceValue > comparisonValue


That way nothing breaks if the enum members are reordered.

Code Snippets

Public Enum comparisonOperator
    '/ Used [[ ]] because the outer set escape the inner brackets for the Enum list, which will then escape the operator when chosen as a value. E.G. they will appear as "Operator = [<]" in code.
    [[<]] = 1
    [[<=]] = 2
    [[=]] = 3
    [[>=]] = 4
    [[>]] = 5
End Enum
Public Enum ComparisonOperator
    EqualTo = 0
    LessThan 
    LessThanOrEqualTo 
    GreaterThanOrEqualTo 
    GreaterThan 
End Enum
Case ComparisonOperator.LessThan
    isTrue = sourceValue < comparisonValue

    Case ComparisonOperator.LessThanOrEqualTo
    isTrue = sourceValue <= comparisonValue

    Case ComparisonOperator.EqualTo
    isTrue = sourceValue = comparisonValue

    Case ComparisonOperator.GreaterThanOrEqualTo
    isTrue = sourceValue >= comparisonValue

    Case ComparisonOperator.GreaterThan
    isTrue = sourceValue > comparisonValue

Context

StackExchange Code Review Q#117856, answer score: 6

Revisions (0)

No revisions yet.