Is this a Best Practice with Enum in C#

Question

When Enum is used as below, say if we have

enum Designation
{
    Manager = 0,
    TeamLead = 1,
    Associate = 2
}

then write the below code

if (designation == Designation.TeamLead) //somecode

Now if we decide to change the enum element from "TeamLead" to "Lead" then we have to modify the above line of code as well i.e. designation == Designation.TeamLead to designation == Designation.Lead. So what is the best practice.

Not exactly relevant to what you're asking, but a best practice is to have `0` as `None` — superM, May 13 '13 at 14:03
Best practice with regard to what? What is the question? What are you concerned about? — Konrad Morawski, May 13 '13 at 14:14
Do you mean "is it a best practice to use enums" in general? You don't appear to be doing anything specific with them, just generally..using them... so, is that all you're curious about? — Jimmy Hoffa, May 13 '13 at 14:18
This is why you use `enum` so you **CAN** rename them. VS renaming tools work just fine on `enum`. — Reactgular, May 13 '13 at 14:47
Why do you explicitly assign the values `0`, `1`, and `2` (which are the default values anyway)? Are you going to write code that depends on `Associate` having the value 2? What would you lose by writing `enum Designation { Manager, TeamLead, Associate }`? — Keith Thompson, May 13 '13 at 17:18
You may not be, but this question makes me feel like you're using the *name* of the enum as the *displayed value* for some property. IE, `designation.ToString()` somewhere in your UI. This is absolutely bad practice. — dlras2, May 13 '13 at 20:21

Katana314 · Accepted Answer · 2013-05-13T14:35:13.330

24

The most basic answer to your question is this: Most C# IDEs I know of have a Refactoring option that easily lets you rename variables and references, such as enum types. If you highlight TeamLead, anywhere it's used, right-click it and look for a Rename option, you should be able to change references in all code files of your project. If you reference it as a string at some point, it may be good to do a full text search and handle individual cases.

However, my full answer, which I can only hope you care about, is that this is the wrong way of going about things in an object-oriented language. You should especially take note of times when you're seperating large blocks of logic by an if-else/switch, and even using part of one block in another, as a time when object-oriented design would help you. Here's how I'd do it, in incomplete pseudo-code:

abstract class ProjectMember {
    public abstract void reassignTask(Task t);
}

class Manager : ProjectMember {
    public void reassignTask(Task t) {
        // TODO: manager case
    }
}

class TeamLead : ProjectMember {
    public void reassignTask(Task t) {
        // TODO: TeamLead case
    }
}

class Associate : ProjectMember {
    public void reassignTask(Task t) {
        // TODO: associate case
    }
}


// this is the function where you'd be getting rid of a switch statement, by way of the above code.
void reassignMemberTask(ProjectMember mem, Task t) {
    mem.reassignTask(t);
}

edited May 13 '13 at 14:35

answered May 13 '13 at 14:20

Katana314

862
1
5
18

This is *way* overkill for this scenario. It might also be entirely infeasible if there's also an enum representing what team the user is on, and the logic is based on the combination thereof. It certainly *could* be the right answer (and the OP accepted it), but it isn't always. – Bobson May 13 '13 at 15:19
1

Well, then you'd have a Team class, and an instance of that in your ProjectMember class. Then, you can have code specific to that Team. I was also going under the assumption that this was hardly the **only** enum-selecting piece of code they were going to have. It's especially useful if you have, for instance, a TeamLeadAssistant class that's just a tiny functional modification of TeamLead. If the project uses these often, then it becomes less and less of an overkill. – Katana314 May 13 '13 at 15:30
1

switch statements are procedural overkill more often than not. Object dispatch exists exactly for this reason, but there needs to be sufficient amount of code in the different switch cases (or sufficient proof they are a part of the system that will surely grow and change with time) to prove necessity for an object hierarchy. This *usually* is the right way to go though. – Jimmy Hoffa May 13 '13 at 15:39
2

@Katana314 Nicely done. I agree with your approach. Too often I have seen methods such as `reassignTask` that contain massive switch statements or if/else that evaluate every permutation of an enum. – Chuck Conway May 13 '13 at 15:42
1

By the time you've got a number of switch statements it's too risky to change the switch statements (there is something to be said for working code, testable or not). If the application is not a proof of concept (POF) or not a throw away application us an object hierarchy. – Chuck Conway May 13 '13 at 15:54
@Katana314 - Fair. I hadn't considered the implication that if there was logic tied to team, then `Team` could be abstracted to its own class rather than being an enum. Although it still gets messy if the logic is tied to the *combination* of them. – Bobson May 13 '13 at 19:04
@Katana314 thanks! Good advice and I adopted this approach as well. – CodingYoshi Feb 06 '17 at 16:36

score 7 · Answer 2 · edited Apr 12 '17 at 07:31

Renaming an element of an enum is not a problem: Visual Studio does that for you, while ensuring that every reference to it will be renamed as well.

On the other hand, other issues may arise when "reusing" the elements in such way:

Business behavior

Imagine that TeamLead is a team lead, while Lead is a lead developer in a team. Imagine the following piece of code:
```
if (role != Designation.TeamLead)
{
    // The user cannot fire a member of a team: only a team lead can do it.
    this.DenyFiringTeamMember();
}
```
If you rename the role, you will miss the business rules, i.e. the fact that only a team lead can fire a team member. Now, lead developer can fire other developers, while only team lead is expected to be able to do it.
Dependencies

Unless you're Google, you may have dependencies in external projects. Visual Studio won't be able to find those, so the problem will arise the next time you open a solution which is referencing the old TeamLead.
ToString

Sometimes, and this is an extremely bad practice, the item from the enum is converted to string, and then used in a comparison, or stored in database, etc. Example:
```
if (role.ToString() == "TeamLead")
{
    // I never understood how to compare variables, and I don't care.
}
```
Renaming the element will break those pieces of code. Maybe it's better: you'll be able to find them and rewrite them correctly.

Point 3: to do simple string comparisons, the issue can be resolved writing `Designation.TeamLead.ToString()` instead of `"TeamLead"`, so when you refactor, the new value is updated automatically and the comparison works as before. The issue remains if you store the string in a database (a file, a sql database, ecc), but maybe you better store the int value beyond the enum, not the string representation... Do you agree? — Massimiliano Kraus, May 31 '16 at 10:17
@MK87: I agree completely with your both points. Storing enum values in a database is indeed problematic; if you use integers, this could lead to another problem of adding values in the middle of an enum or removing a value from the middle (shifting the other values). I think the *only* sane way is to have a converter which binds the enum values (identified as `Designation.TeamLead`, and not a string or an int) to the corresponding meaningful values from the database—`varchar`s or maybe numbers specified as constants in code. — Arseni Mourzenko, May 31 '16 at 11:51
Yep, a bridge/converter/facade/mediator between the enum and the db seems the most solid solution to me too. — Massimiliano Kraus, May 31 '16 at 16:19

score 1 · Answer 3 · answered May 13 '13 at 14:16

1

Just use a refactoring tool. You cannot circumvent the fact that if you rename something you have to rename it everywhere but a refactoring tool will make that easy.

answered May 13 '13 at 14:16

Sarien

751
4
13

Why the down vote? – Sarien May 13 '13 at 14:25
1

+1 I don't know why you got downvoted. It's a reasonable answer. – Chuck Conway May 13 '13 at 15:46
@ChuckConway well - I have not downvoted it, but it doesn't really answer the question. The question is poorly phrased anyhow, but it's clearly asking about best practices (the author just failed to specify what he is concerned about). He's asking about what he should do, not how. – Konrad Morawski May 14 '13 at 11:34

score 1 · Answer 4 · answered May 13 '13 at 14:19

1

Most refactoring tools can easily help with this. It allows mass renaming based on language semantics, not only text-replace.

But if you use complex enums for business logic, I would start thinking if you shouldn't be using proper OOP.

answered May 13 '13 at 14:19

Euphoric

36,735
6
78
110

+1 I don't know why you got downvoted. It's a reasonable answer. – Chuck Conway May 13 '13 at 15:48

score 1 · Answer 5 · edited Apr 12 '17 at 07:31

I will answer, not because I think the other answers are bad, but because I think there is another thing to be said in this case.

We have two questions in one:

About best practice for you Enum
About refactoring an Enum

1)Is this a Best Practice with Enum in C#?

Well, in your case, since you Enum seems to be kind of hierarchic, I would define them as flags, and benefit from that when using and comparing the values.

[Flags]
enum Designation
{
    None = 0x0,
    TeamLead = 0x1,
    Manager = 0x2,
    Associate = 0x4
}

Then you could use a code like this:

    string CheckAboveManager(Designation designation)
    {
        if (designation >= Designation.Manager)
            return "It's a manager or higher";
        return "Is lower than manager";
    }

The other option of using a object-oriented structure will work only if there is specific behavior or structure you want to separate concerns with. If it is a case of just defining a property (that why there are enums!), them I see no need to create inheritance, but if it is the case, use the accepted answer.

2) Now if we decide to change the enum element from "TeamLead" to "Lead" then we have to modify the above line of code as well i.e. designation == Designation.TeamLead to designation == Designation.Lead

Well, it was already well explained that refactoring is not an issue if you are using VS IDE, you can even use other tools like DevExpress CodeRush! or ReSharper to get an even better (and very safe) result from refactoring.

When adding the Flags attribute to a enum it is best practice to make the enum name plural, like Designations in this case. Defining a default value is a good idea because comparing your enum with default(value) should result in the most cases to a invalid/unknown value. — seveves, Feb 16 '16 at 07:49

score 0 · Answer 6 · answered May 14 '13 at 05:47

0

If you rename the enum member in a DLL, the caller code needs to be modified and recompiled.

This may break binary compatibility. You need to ensure the caller code is possible to be modified.

answered May 14 '13 at 05:47

linquize

1,150
1
11
20

Only relevant if the enum is public. – Marcel May 14 '13 at 06:37

Is this a Best Practice with Enum in C#

6 Answers6

Linked