Bad practice with Enumerations....

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 )
 
C

Chris Dunaway

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

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
 
J

Jay B. Harlow

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

FirstValue = 0
SecondValue
ThirdValue

MaxValue = ThirdValue

End Enum

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 )
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))
 
R

Robinson

Otherwise MaxValue would be one more then the actual MaxValue!


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.

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.

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...

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

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Top