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;}
}
Why not to use Set? If using List:
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.
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.
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
}
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?
Should I use some other class?
Is the Role enum list sufficient for use in User?
WTF Bob?
This is a design question that is framed in programming language technicalities.
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. "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.
Role property that tells what kind of role this class-instance is.User.Roles list is reasonable when we don't need, or don't want, instantiated role objects.RoleFactory class; and, not insignificantly, to the code reader.
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.
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.
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.
