CODEWITHSUNDEEP
c#architecturesolid-principles
Lukasz
Lukasz

Reputation: 8900

Architecture for Avoiding Switch Statements

I am working on a new project and I am trying to adhere to proper design methods. I have ran into an issue with a switch statement that I know is a problem but I am unable to re-factor it out in an object oriented way.

In the system a User has 0..n roles. Based on which role the user is at the moment the system will return a specific set of data to that user. The user will be able to perform certain actions but not others etc.

public class User
{
    public bool HasRole(string roleName)
    {
        return this.UserRoles.Any(r => r.Name == roleName);
    }

    public string Username { get; set; }

    public long? CurrentRoleId { get; set; }
    public Role CurrentRole { get; set; }

    public virtual IList<Role> UserRoles { get; set; }
}

public class Role
{
    public Role() { }

    public string Name { get; set; }
}

public class GetEventsQuery : IQuery<List<Event>>
{
    public List<Event> Query()
    {
        switch (this.user.CurrentRole.Name)
        {
            case "Administrator":
                UserIsNotInAdministratorRole();
                return repository.All.ToList();

            case "Supervisor":
                UserIsNotInSupervisorRole();
                return repository.All.Where(evnt => evnt.SupervisorId == this.user.RecordId).ToList();

            case "User":
                UserIsNotInUserRole();
                return repository.All.Where(evnt => evnt.UserId == this.user.RecordId).ToList();

            default:
                throw new Exception("GetEventsQuery Unknow exception.");
        }
    }

    private void UserIsNotInUserRole()
    {
        if (!this.user.HasRole("User"))
        {
            throw new NoUserRoleException("User does not have user role!");
        }
    }

    private void UserIsNotInSupervisorRole()
    {
        if (!this.user.HasRole("Supervisor"))
        {
            throw new NoSupervisorRoleException("User does not have supervisor role!");
        }
    }

    private void UserIsNotInAdministratorRole()
    {
        if (!this.user.HasRole("Administrator"))
        {
            throw new NoAdministratorRoleException("User does not have administrator role!");
        }
    }

    public GetEventsQuery(string username, IUserRepository userRepository, IEventRepository repository)
    {
        this.repository = repository;

        var userQuery = new GetUserQuery(username, userRepository);
        this.user = userQuery.Query();
    }

    private readonly User user;
    private readonly IEventRepository repository;
}

This switch statement will show up in all parts of the system. If there was a way to re-factor it out into a class and have it in a single location I don't mind keeping it but having it repeated over and over again is definitely a code smell. I am just starting this project out so if there is a better way to design the object hierarchy or make major changes to eliminate this issue I am open to it.

Upvotes: 3

Views: 1165

Answers (5)

MoonKnight
MoonKnight

Reputation: 23831

What you want in this case is a combination of the Factory and Strategy patterns.

The Strategy pattern defines a family of algorithms, encapsulates each one, and makes them interchangeable. Strategy lets the algorithm vary independently from the clients that use it.

The Factory pattern provides an interface for creating families of related or dependent objects without specifying their concrete classes.

When the above are used together you will be able to create a fully extensible class that will provide all the functionality you want...

I hope this helps.

Upvotes: 1

Rik
Rik

Reputation: 29243

Role should be a common base class (or interface) inherited by each role (e.g Administrator, Supervisor, User.) Each implementation of Role should have them information on what is or is not allowed for that particular role.

Upvotes: 3

Vishy
Vishy

Reputation: 1362

Interface IRole:

GetRole(); //You can use it to self-define the role
IsUserInRole(Role roleType) //Check if user in specific role
GetList(); //Query

Define specific Roles:

SuperVisor implements IRole
{...}

Administrator implements IRole
{...}

Advantage here is that you can rely on IRole to maintain your transaction instead of dealing with direct Role objects (AdministratorObject) and cast it to appropriate class when you need to call any specific method inside that class's definition.

Upvotes: 2

Simon Martin
Simon Martin

Reputation: 4231

Can you use polymorphism to avoid your switch?
You would use an abstraction to contain your behaviour as necessary. And then create concrete implementations of that which encapsulate the logic specific to them - these then replace your switch statement. It would be the Role that needed to be refactored out, so possibly something like

public interface IRole
{
   // whatever you need here
}

public class Administrator : IRole
{
    // your implementation 
}

There's a good post on lostechies.com on polymorphism-part-2-refactoring-to-polymorphic-behavior

Upvotes: 3

Joe Ratzer
Joe Ratzer

Reputation: 18569

The Strategy Pattern is often a good pattern to use when replacing non-OO switch statements.

Upvotes: 4

Related Questions