Sometimes an enum is not the best idea

In our application we are dealing with a limited, but slowly growing number of suppliers. In some parts of the code each one is treated differently, in others parts we only have to identify them. Their id which is also understood by the outer world, is a simple integer number. At first sight a nice and clean solution would be to use an enum.

public class Supplier

{

public enum PublicIds

{

Company1 = 1,

MyCompany = 5,

ThatOtherCompany = 7

}

All code can now use quite these descriptive identifiers.

public void DoWithCompany(PublicIds company)

{

switch (company)

{

case PublicIds.Company1:

// Do something with Company 1

break;

case PublicIds.MyCompany:

// Do something with My Company

break;

case PublicIds.ThatOtherCompany:

// Do something else

break;

}

}

<Update>

Note that this DoWithCompany method is not a member of the Company class. That would indeed be, as many a commenter has noted, be bad OO design. This method is a member of several quite different other classes and comes in many flavors. The suggestion to make the dowith behavior a polymorphic member of a Company would drag a lot of behavior into the company which does not belong there.

This company is a simple class with a simple ID member. The best type for this ID is an enum, this post is only about solving a problem with such an enum.

</Update>

But there are situations where an enum will hit back. In case a parameter of type object is expected the value will be that of the string representation of the enum's member. For instance when you want to use the value as a parameter in a sql query.

cmdExists.Parameters.AddWithValue("@idSupplier", Supplier.PublicIds.MyCompany);

At first sight you are passing in the value 5 to the query. In reality you are passing in 'MyCompany', the ToString() representation of Supplier.PublicIds.MyCompany. This will result in a quite different query result as perhaps expected.

One workaround is to cast the enum member to the intended type.

cmdExists.Parameters.AddWithValue("@idSupplier", (int) Supplier.PublicIds.MyCompany);

Which has the risk of forgetting to do that.

Another solution is to drop the enum. Which can be done without losing the clarity of the code. It will still work using this class instead of the enum

public class PublicIds

{

public const int Company1 = 1;

public const int MyCompany = 5;

public const int ThatOtherCompany = 7;

}

The only difference is the type of the parameter of the method, which is now an int instead on the enum type

public void DoWithCompany(int company)

{

switch (company)

{

case PublicIds.Company1:

// Do something with Company 1

break;

case PublicIds.MyCompany:

// Do something with My Company

break;

case PublicIds.ThatOtherCompany:

// Do something else

break;

}

}

You lose the type checking. When the company still was of type PublicId any call not passing in a member of that enum would throw an exception. Now any int value is accepted.

But you gain the certainty that the Id passed always has the intended value. Without sacrificing the readability of the code.