How would you write this block of code?

Question

How would you write this block of code?

A user can have 3 levels of permission:

Suspend Lock Ban

We have to decide if we want to delete the users account automatically.

The business rules are as follows:

Delete User if: suspend + lock + ban

Warning Message if: suspend + lock

So how would you write this code block? Keeping code readability and maintainability in mind.

Answer 1

I would create 3 predicates, like

Boolean canLoginIntoSuperUser()
Boolean canLoginIntoManagerSession()

And then then put the code for a check (e.g. return isOwner() && isManager() && isManager for the superuser).

This way you got the actual check just in one place which makes it maintainable and the referring checks are very readable.

Offcourse you could create a enum with the LoginSections, but there it would it not be possible to have different states for Manager Section and Super User section (i.e. every Super User Section user has implicit access to the Manager Section, maybe you want to change that later).

Answer 2

Usually it would be a case where you would just use if, elseif, elseif, else statements.

But I would just not code permissions like that inside a software. It is a lot easier to think in features in a software and roles should reflect features. So:

A Role data structure. (the features) A RoleGroup data structure. (to map stuff like user, manager, etc. to features)

Then it becomes very easy, you won't have to code switches everywhere, just a simple if.

Adding to that, a lot of people will also use bit wise operations on bit fields.

Answer 3

I'd write this:

[Flags]
public enum UserLevel {
    None = 0,
    Suspend = 1,
    Lock = 2,
    Ban = 3
};

public class UserStatus {
    UserLevel _level;

    public bool IsDeletable {
        get {
            return Suspended && Locked && Banned;
        }
    }
    public bool Suspended {
        get { return (_level & UserLevel.Suspend) != 0; }
        set { _level = (_level  & ~UserLevel.Suspend) | value ? UserLevel.Suspend : UserLevel.None; }
    }
    // etc for other props

}

Answer 4

something like

[Flags()]
enum Things
{
 Suspend,Lock,Ban
}

public Foo(Things userFlags)
{
 switch(userFlags)
 {
  case Things.Suspend & Things.Lock & Things.Ban:
   //delete
   break;
  case Things.Suspend & Things.Ban:
   //somethign else
   break;
  // yada yada yada

 }
}

This is something you can grow with over time and is more readable

Answer 5

I would create the following enum:

[Flags]enum UserLevel 
{ None=0, Suspend=1, Lock=2, Ban=4, 
  Delete=Suspend | Lock | Ban,  // equililent to 7
  Warn=Suspend | Lock }         // equililent to 3

Then you can use bitwise enum comparisons

if ((user.Level & UserLevel.Warn) == UserLevel.Warn) 
     user.Warn();
else if ((user.Level & UserLevel.Delete) == UserLevel.Delete) 
     user.Delete();

Answer 6

keithwarren7 had the best answer, although he missed two key points: First, the enums need to be set to bit position values. Next the values for the case statement must be added or OR'd. If they are AND'd they'll all result in zero:

case Things.Suspend | Things.Lock | Things.Ban:

Putting that with some other good ideas we have here, we get:

[Flags]
enum Things
{
 None = 0,
 Suspend = 1,
 Lock= 2,
 Ban = 4,
 Delete = Suspend | Lock | Ban,  
 Warn = Suspend | Lock   
}

public bool IsDeletable(this User user)
{ 
    return ((user.Level & UserLevel.Delete) == UserLevel.Delete) ;
}
// :

if (user.IsDeletable)
    user.Delete();

(Of course, that kind-of assumes you are using C#3, when you never actually mention which language you were using....)

Answer 7

A lot of people are fond of bitfields. I am not. I believe the solution to be more readable and maintainable when you use simple boolean properties.

class User 
{
    public bool IsSuspended { get; set; }
    public bool IsLocked { get; set; }
    public bool IsBanned { get; set; }
    public bool ShouldDelete { get { return IsSuspended && IsLocked && IsBanned; } }
    public bool ShouldWarn { get { return IsSuspended && IsLocked; } }
}