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

EF lookups and enums

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

Problem

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?

Solution

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

    //......
}

Code Snippets

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

switch(currentRequestType )
{
    case RequestTypeEnum.Leave:

        // request is leave
        break;

    case RequestTypeEnum.Loan:

        // request is loan
        break;

    //......
}

Context

StackExchange Code Review Q#92968, answer score: 5

Revisions (0)

No revisions yet.