Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I am currently working on a system where there are Users, and each user have one or multiple roles. Is it a good practice to use List of Enum values on User? I can't think of anything better, but this doesn't feel alright.

enum Role{
  Admin = 1,
  User = 2,
}

class User{
   ...
   List<Role> Roles {get;set;}
}
share|improve this question
5  
Looks fine to me, i'd be interested to see anyone else's comments to the contrary. – David Scholefield Jan 21 at 9:30
8  
@MatthewRock thats a pretty sweeping generalization. List<T> is quite common in the .NET world. – Graham Jan 21 at 14:13
6  
@MatthewRock .NET List is an arraylist, which has the same properties for the algorithms you mention. – im so confused Jan 21 at 14:33
17  
@MatthewRock - no, you are talking about LINKED lists, when the question, and everyone else, is talking about generic List interface. – Davor Ždralo Jan 21 at 17:54
5  
Auto properties are a distinctive feature of C# ( the get; set; syntax). Also the List class naming. – jaypb Jan 22 at 17:30
up vote 78 down vote accepted

Why not to use Set? If using List:

  1. It is easy to add the same role twice
  2. Naive compare of lists won't work here properly: [User, Admin] is not the same as [Admin, User]
  3. Complex operations such as intersect and merge are not straightforward to implement

If you are concerned about performance, then, for example in Java, there is EnumSet which is implemented as fixed length array of booleans (i'th boolean element answers the question whether i'th Enum value is present in this set or not). For example, EnumSet<Role>. Also, see EnumMap. I suspect that C# has something similar.

share|improve this answer
34  
Actually in Java EnumSets are implemented as bit fields. – biziclop Jan 21 at 13:10
3  
Related SO question on HashSet<MyEnum> vs. flags enums: stackoverflow.com/q/9077487/87698 – Heinzi Jan 21 at 18:51
    
HashSet is exactly what I am looking for! Thanks! – Dexie Jan 22 at 9:47
3  
.NET has FlagsAttribute, which lets you use bitwise operators to concat enums. Sounds similar to the Java EnumSet. msdn.microsoft.com/en-US/LIBRARY/system.flagsattribute EDIT: I should've looked down one answer! it has a good example of this. – ps2goat Jan 22 at 15:17

TL;DR: It is usually a bad idea to use a collection of enums as it often leads to a bad design. A collection of enums usually calls for distinct system entities with specific logic.

It is necessary to distinguish between a few use cases of enum. This list is only of top of my head so there might be more cases...

The examples are all in C#, I guess your language of choice will have similar constructs or it would be possible for you to implement them yourself.

1. Only single value is valid

In this case, the values are exclusive, e.g.

public enum WorkStates
{
    Init,
    Pending,
    Done
}

It is invalid to have some work that is both Pending and Done. Therefore only one of these values is valid. This is a good use-case of enum.

2. A combination of values is valid
This case is also called flags, C# provides [Flags] enum attribute to working with these. The idea can be modeled as a set of bools or bits with each corresponding to one enum member. Each member should have a value of power of two. Combinations can be created using bitwise operators:

[Flags]
public enum Flags
{
    None = 0,
    Flag0 = 1, // 0x01, 1 << 0
    Flag1 = 2, // 0x02, 1 << 1
    Flag2 = 4, // 0x04, 1 << 2
    Flag3 = 8, // 0x08, 1 << 3
    Flag4 = 16, // 0x10, 1 << 4

    AFrequentlyUsedMask = Flag1 | Flag2 | Flag4,
    All = ~0 // bitwise negation of zero is all ones
}

Using a collection of enum members is an overkill in such a case as each enum member only represents one bit that is either set or unset. I guess most of languages support constructs like this. Otherwise you can create one (e.g. use bool[] and address it by (1 << (int)YourEnum.SomeMember) - 1).

a) All combinations are valid

While these are ok in some simple cases, a collection of objects may be more appropriate as you often need additional information or behaviour based on the type.

[Flags]
public enum Flavors
{
    Strawberry = 1,
    Vanilla = 2,
    Chocolate = 4
}

public class IceCream
{
    private Flavors _scoopFlavors;

    public IceCream(Flavors scoopFlavors)
    {
        _scoopFlavors = scoopFlavors
    }

    public bool HasFlavor(Flavors flavor)
    {
        return _scoopFlavors.HasFlag(flavor);
    }
}

(note: this assumes you only really care about the flavors of the ice-cream - that you don't need to model the ice-cream as a collection of scoops and a cone)

b) Some combinations of values are valid and some not

This is a frequent scenario. The case might often be that you're putting two different things into one enum. Example:

[Flags]
public enum Parts
{
    Wheel = 1,
    Window = 2,
    Door = 4,
}

public class Building
{
    public Parts parts { get; set; }
}

public class Vehicle
{
    public Parts parts { get; set; }
}

Now while it's completely valid for both Vehicle and Building to have Doors and Windows, it's not very usual for Buildings to have Wheels.

In this case, it would be better to break up the enum into parts and/or modify the object hierarchy in order to achieve either case #1 or #2a).

Design considerations

Somehow, the enums tend not to be the driving elements in OO since the type of an entity can be considered similar to the information that is usually provided by an enum.

Take e.g. the IceCream sample from #2, the IceCream entity would instead of flags have a collection of Scoop objects.

The less purist approach would be for the Scoop to have a Flavor property. The purist approach would be for the Scoop to be an abstract base class for VanillaScoop, ChocolateScoop, ... classes instead.

The bottom line is that:
1. Not everything that is a "type of something" needs to be enum
2. When some enum member is not a valid flag in some scenario, consider splitting the enum into multiple distinct enums.

Now for your example (slightly changed):

public enum Role
{
    User,
    Admin
}

public class User
{
    public List<Role> Roles { get; set; }
}

I think this exact case should be modeled as (note: not really extensible!):

public class User
{
    public bool IsAdmin { get; set; }
}

In other words - it is implicit, that the User is a User, the additional info is whether he is an Admin.

If you get to have multiple roles that are not exclusive (e.g. User can be Admin, Moderator, VIP, ... at the same time), that would be a good time to use either flags enum or an abtract base class or interface.

Using a class to represent a Role leads to a better separation of responsibilities where a Role can have the responsibility to decide whether it can do a given action.

With an enum you'd need to have the logic in one place for all the roles. Which defeats the purpose of OO and brings you back to imperative.

Imagine that a Moderator has edit rights and Admin has both edit and delete rights.

Enum approach (called Permissions in order not to mix roles and permissions):

[Flags]
public enum Permissions
{
    None = 0
    CanEdit = 1,
    CanDelete = 2,

    ModeratorPermissions = CanEdit,
    AdminPermissions = ModeratorPermissions | CanDelete
}

public class User
{
    private Permissions _permissions;

    public bool CanExecute(IAction action)
    {
        if (action.Type == ActionType.Edit && _permissions.HasFlag(Permissions.CanEdit))
        {
            return true;
        }

        if (action.Type == ActionType.Delete && _permissions.HasFlag(Permissions.CanDelete))
        {
            return true;
        }

        return false;
    }
}

Class approach (this is far from perfect, ideally, you'd want the IAction in a visitor pattern but this post is enormous already...):

public interface IRole
{
    bool CanExecute(IAction action);
}

public class ModeratorRole : IRole
{
    public virtual bool CanExecute(IAction action)
    {
         return action.Type == ActionType.Edit;
    }
}

public class AdminRole : ModeratorRole
{
     public override bool CanExecute(IAction action)
     {
         return base.CanExecute(action) || action.Type == ActionType.Delete;
     }
}

public class User
{
    private List<IRole> _roles;

    public bool CanExecute(IAction action)
    {
        _roles.Any(x => x.CanExecute(action));
    }
}

Using an enum may be an acceptable approach though (e.g. performance). The decision here depends on the requirements of the modeled system.

share|improve this answer
2  
I don't think that [Flags] implies all combinations are valid any more than an int implies all four billion values of that type are valid. It simply means they can be combined within a single field - any restrictions on combinations belongs to higher-level logic. – Random832 Jan 21 at 15:42
    
Warning: when using [Flags], you should set the values to powers of two, or else it won't work as expected. – Arturo Torres Sánchez Jan 21 at 16:20
    
@Random832 I never had an intent of saying that but I edited the answer - hope it's clearer now. – Zdeněk Jelínek Jan 21 at 17:36
1  
@ArturoTorresSánchez Thank you for the input, I have fixed the answer and also added an explanatory note about that. – Zdeněk Jelínek Jan 21 at 17:36
    
Great explanation! Actually flags fit system requirements very-well, however it will be easier to use HashSets due to existing service implementation. – Dexie Jan 22 at 9:54

Write your enum so you can combine them. By using base 2 exponential you can combine them all in one enum, have 1 property and can check for them. Your enum should be lile this

enum MyEnum
{ 
    FIRST_CHOICE = 2,
    SECOND_CHOICE = 4,
    THIRD_CHOICE = 8
}
share|improve this answer
3  
Any reason why you're not using 1? – Robbie Dee Jan 21 at 12:27
1  
@RobbieDee no effectivly I could have used 1, 2, 4, 8, etc. you're right – Rémi Jan 21 at 13:00
5  
Well, if you use Java, it already has EnumSet which implements this for enums. You have all the boolean arithmetic already abstracted, plus some other methods to make it easier to use, plus small memory and good performance. – fr13d Jan 21 at 14:43
2  
@fr13d I'm a c# guy and since the question has no java tag I think this answer apply more at large – Rémi Jan 21 at 14:45
1  
Right, @Rémi. C# provides the [flags] attribute, which @Zdeněk Jelínek explores in his post. – fr13d Jan 21 at 15:19

Is it a good practice to use List of Enum values on User?


Short answer: Yes


Better short answer: Yes, the enum defines something in the domain.


Design-time answer: Make and use classes, structures, etc. that model the domain in terms of the domain itself.


Coding time answer: Here's how to code-sling enums ...


The inferred questions:

  • Should I use strings?

    • answer: No.
  • Should I use some other class?

    • In addition to, yes. Instead of, no.
  • Is the Role enum list sufficient for use in User?

    • I have no idea. It is highly dependent on other design details not in evidence.

WTF Bob?

  • This is a design question that is framed in programming language technicalities.

    • An enum is a good way to define "Roles" in your model. So then a List of roles is a good thing.
  • enum is far superior to strings.

    • Role is a declaration of ALL roles that exist in the domain.
    • The string "Admin" is a string with the letters "A" "d" "m" "i" "n", in that order. It is nothing as far as the domain is concerned.
  • Any classes you might design to give the concept of Role some life - functionality - can make good use of the Role enum.

    • For example rather than sub-class for every kind of role, have a Role property that tells what kind of role this class-instance is.
    • A User.Roles list is reasonable when we don't need, or don't want, instantiated role objects.
    • And when we do need to instantiate a role, the enum is an un-ambiguous, type safe way to communicate that to a RoleFactory class; and, not insignificantly, to the code reader.
share|improve this answer

That isn't something I've seen but I see no reason why it wouldn't work. You might want to use sorted list so it defends against dupes however.

A more common approach is a single user level e.g. system, admin, power user, user etc. System can do everything, admin most things and so on down.

If you were to set the values of roles as powers of two, you could store the role as an int and derive the roles if you really wanted to store them in a single field but that might not be to everyone's taste.

So you might have:

Back office role 1
Front office role 2
Warehouse role 4
Systems role 8

So if a user had a role value of 6 then they'd have the Front office and Warehouse roles.

share|improve this answer

Use HashSet as it prevents duplicates and has more comparison methods

Otherwise good use of Enum

Add a method Boolean IsInRole(Role R)

and I would skip the set

List<Role> roles = new List<Role>();
Public List<Role> Roles { get {return roles;} }
share|improve this answer

The simplistic example you give is fine, but typically it's more complicated than that. For instance, if you are going to persist the users and roles in a database, then you should define the roles in a database table rather than use enums. That gives you referential integrity.

share|improve this answer

If a system — might it be a software, an operation system or an organization — deals with roles and permission, there comes a time when it is useful to add roles and manage permissions for them.
If this can be archived by altering the source code, it might be ok, to stick with enums. But at some point the system's user wants to be able to manage roles and permissions. Than enums become unusable.
Instead you will want to have a configurable data structure. i.e. a UserRole class that also holds a set of permission assigned to the role.
A User class would have a set of roles. If there is the need to check user's permission, a union set of all roles permissions is created and checked if it contains the permission in question.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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