6
\$\begingroup\$

I have a lookup table "RequestTypes" that has the following data:

ID      Type
----   -----

1      Leave
2      Loan
3      BlahBlah

Using EF, and whenever the need arise to check the type of the request, I used to do something like:

if (row.RequestType.ID == 1)
{
     // request is leave
}
else if (row.RequestType.ID == 2)
{
    // request is loan
}
//......

As you can see, there are lots of magic numbers. Anyway, I then created an enum that matches the lookup values:

public enum RequestTypesEumn
{
     Leave = 1,
     Loan = 2,
     BlahBlah = 3
}

and when I need to check the request type, I do something like:

if ((RequestTypesEumn)row.RequestType.ID == RequestTypesEumn.Leave)
{
     // request is leave
}
else if ((RequestTypesEumn)row.RequestType.ID == RequestTypesEumn.Loan)
{
    // request is loan
}
//......

That's a little bit better, except it is a lot of casting. So I created a partial class of the Request object and overloaded the == and != operators to match it against the automatically generated virtual property RequestType:

public static bool operator ==(RequestType type, RequestTypesEumn typeEnum)
{
    if (type== null)
       return false;

    return (type.ID == (int)typeEnum);
}

public static bool operator !=(RequestType type, RequestTypesEumn typeEnum)
{
    if (type== null)
       return true;

    return (type.ID != (int)typeEnum);
}

So now I can simply do:

if (row.RequestType == RequestTypesEumn.Leave)
{
     // request is leave
}
else if (row.RequestType == RequestTypesEumn.Loan)
{
    // request is loan
}
//......

Let alone inline code in ASP.NET as in repeaters and grids, for example to change the row color depending on the type we'll need to check the type and so on.

Is that a good practice? is it the best possible way to deal with enums especially when the logic requires a lot of matching as in my case?

\$\endgroup\$
  • 2
    \$\begingroup\$ It is an interesting solution, though practically I would rather choose @Heslacher 's solution for better readablity. \$\endgroup\$ – Vojtěch Dohnal Jun 8 '15 at 6:54
5
\$\begingroup\$
  • Instead of overloading the == and != operator you could simplify the part with the heavy casting by casting only once.

  • Instead of multiple if..else..else if statements you should use a switch..case.

  • using braces {} for single statment if's will make your code less error prone.

  • also this will be just a typo, but you really should fix it. RequestTypesEumn vs. RequestTypesEnum this can be easily achieved if you place the cursor on the name of the enum and hit the F2 key. And while we are at this enum, the enum in the name automatically implies that these are multiple items, so RequestTypeEnum instead of RequestTypesEnum would be better.


RequestTypeEnum currentRequestType = (RequestTypesEumn)row.RequestType.ID;

switch(currentRequestType )
{
    case RequestTypeEnum.Leave:

        // request is leave
        break;

    case RequestTypeEnum.Loan:

        // request is loan
        break;

    //......
}
\$\endgroup\$
  • \$\begingroup\$ Casting once is ok,but how about inline coding in ASP.NET? \$\endgroup\$ – Nean Der Thal Jun 8 '15 at 6:54
  • \$\begingroup\$ I don't see anywhere ASP.NET mentioned. \$\endgroup\$ – Heslacher Jun 8 '15 at 7:06
  • \$\begingroup\$ I edited my question a bit, sorry for that. \$\endgroup\$ – Nean Der Thal Jun 8 '15 at 7:11
  • 1
    \$\begingroup\$ Also: "Do not use an Enum suffix on Enum type names." \$\endgroup\$ – BCdotWEB Jun 10 '15 at 12:00
  • \$\begingroup\$ I can't cover all ;-) @BCdotWEB \$\endgroup\$ – Heslacher Jun 10 '15 at 12:01
2
\$\begingroup\$

You have a deeper issue in that you are not taking advantage of letting EF do the work for you with enums so you're forced to cast int values to their respective enum values.

Here's a link to a video that shows you how to take advantage of enum support in EF using the designer (code first also supported):

https://msdn.microsoft.com/en-us/data/jj248772.aspx

If you can't or don't want to use that approach for some reason, then I recommend taking Heslacher's suggestion to use a switch to clean up all of your inline conditionals. If you don't like ad hoc casting you can create an object with a method that takes in an int and returns it cast to an enum, which could in theory save you future effort if your enum gets repurposed, but for simplicity's sake I'd just stick with the cast.

\$\endgroup\$
  • \$\begingroup\$ I was working with EF 4, then jumped to EF6, I thought it was never implemented. Thanks for the tip. This really saves me from all this headache :) \$\endgroup\$ – Nean Der Thal Jun 8 '15 at 18:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.