Bad practice with Enumerations....

  • Thread starter Thread starter Robinson
  • Start date Start date
R

Robinson

Hi,

I have a vauge recollection that setting a "MAX" member of an enumeration is
bad practice. I can't for the life of me remember or think why this might
be so and would appreciate your thoughts on doing this:


Public Enum MyEnum

FirstValue = 0
SecondValue
ThirdValue

MaxValue

End Enum



.... and perhaps choosing a random value from the enumeration, such as:



Dim theRandom As New Random(1234)

Dim theEnumValue As MyEnum = CType ( theRandom.Next ( 0, MyEnum.MaxValue ),
MyEnum )
 
Robinson said:
I have a vauge recollection that setting a "MAX" member of an enumeration is
bad practice. I can't for the life of me remember or think why this might
be so and would appreciate your thoughts on doing this:

Public Enum MyEnum

FirstValue = 0
SecondValue
ThirdValue

MaxValue

End Enum
Click to expand...

If it makes sense in your object model to have a MaxValue for an
enumeration, I don't see that it is bad practice. I think more common
is an enumeration value that is a combination of one or more values,
especially when the enumeration is used as a set of flags.

<Flags> Public Enum Options
NoOptions = 0
Option1 = 1
Option2 = 2
Option3 = 4
Option4 = 8
AllOptions = 15
End Enum

Often, the 'AllOptions' value is expressed like this:

AllOptions = Option1 And Option2 And Option3 And Option4

Chris
 
Robinson,
I'm sure you meant:
Public Enum MyEnum

FirstValue = 0
SecondValue
ThirdValue

MaxValue = ThirdValue

End Enum
Click to expand...

Otherwise MaxValue would be one more then the actual MaxValue!

The .NET guidelines themselves suggest that you avoid sentinel values in
enums. Unfortunately I don't see an online reference to the reason, it is
however in the printed copy of the book "Framework Design Guidelines -
Conventions, Idioms, and Patterns for Reusable .NET Libraries" by Krzysztof
Cwalina & Brad Abrams from Addison Wesley.

<quote>
4.8 Enum Designs

- Do Not include sentinel values in enums.

although they are sometimes helpful to framework developers, they are
confusing to user of the framework. Sentinel values are values used to track
the state of the enum, rather then being one of the values from the set
represented by the enum.
....
Rather then relying on sentinel value, framework developers should perform
the check using one of the real enum values.

</quote>

In your words, your check should look for ThirdValue itself rather then
MaxValue, likewise you check should look for FirstValue also. (in case a
negative number was converted to the Enum).

The users or your enum (including your predecessors) won't know if MaxValue
should be used or not. Hence you should avoid including it.
Dim theRandom As New Random(1234)

Dim theEnumValue As MyEnum = CType ( theRandom.Next ( 0,
MyEnum.MaxValue ), MyEnum )
Click to expand...
I would use Enum.GetValues to get the list of allowed values, then get a
random index into this list to retrieve a random value. Ensure that it will
work with Enums that are not zero based. Hint: Generally I make FirstValue =
1, and have 0 be a None value, so my code causes an error if I pass an
uninitialized enum value...

Dim theValues() As MyEnum =
DirectCast([Enum].GetValues(GetType(MyEnum)), MyEnum())
Dim theRandom As New Random(1234)

Dim theEnumValue As MyEnum = theValues(theRandom.Next(0,
theValues.Length))
 
Otherwise MaxValue would be one more then the actual MaxValue!
Click to expand...


Actually Jay, the way I use the sentinel value (nice term) is so I can loop
with something like:

For i As Integer = 0 To MyEnum.MaxValue - 1

Next

However, you are right, it's much better if using sentinels to make it equal
the last term in the enum.
Rather then relying on sentinel value, framework developers should perform
the check using one of the real enum values.
Click to expand...

This is fine imho, however during development I'm often moving values about,
adding, removing or renaming them. It's very easy to forget to update the
max value in this case This is another reason I just made MaxValue 1 + the
last value in the Enum.
The users or your enum (including your predecessors) won't know if
MaxValue should be used or not. Hence you should avoid including it.
Click to expand...

Yes this is a good reason not to use it.
I would use Enum.GetValues to get the list of allowed values, then get a
random index into this list to retrieve a random value. Ensure that it
will work with Enums that are not zero based. Hint: Generally I make
FirstValue = 1, and have 0 be a None value, so my code causes an error if
I pass an uninitialized enum value...
Click to expand...

Ah, yes. This is much better. Thanks for the tip.
 
Back
Top