Implementing Strategy pattern instead of several if statements

Question

I have this method with lot of if statements, in which I'm filtering SharePoint list based on employee position.The result is query string which is passed as parameter to another method QuerySPList.

 public List<Phone> GetListOfPhones(Employee emp)
 {
    List<Phone> records = new List<Phone>();
    string query = string.Empty;

    if (emp.Positions.Count == 1 && emp.Positions[0] == "Regular")
    {
       query = "";// some querystring                              
    }


    if (emp.Positions.Count == 1 && emp.Positions[0] == "OfficeDirector")
    {   
       query = "";// some querystring    
    }

    if (emp.Positions.Count == 1 && emp.Positions[0] == "Admin")
    {
        query = "";// some querystring 
    }              

    if (emp.Positions.Count == 2 && emp.Positions.Contains("Regular") && emp.Positions.Contains("OfficeDirector"))
    {

      query = "";// some querystring 

     }

   var rawItems = QuerySPList(query);

   foreach (SPListItem item in rawItems)
   {
        //business logic 
   }
   return records;
}}

I've read that with implementing strategy pattern we can avoid messy code with lot of if's , however , I'm new to developing , and design patterns , so I need little help with this one,so if someone can guide me what should I do and give me advice , feel free to answer my question.I have idea , but I think that I'm going in the wrong direction. Idea is to create interface IRoleHandler and than implement it to 3 classes , for example RoleAdmin ,RoleRegular,RoleOfficeDirector.Something like this :

public interface IRoleHandler<T>
{
    string handleRole(T obj);
}
public class RoleAdmin:IRoleHandler<Employee>
{

    public string handleRole(Employee emp)
    {
        if (emp.Positions.Count == 1 && emp.Positions[0] == "Admin")
        {
            //return some string query
        }

    }
}

Then idea is to create dictionary,something like this :

Dictionary<string, IRoleHandler<Employee>> strategyHandlers = new Dictionary<string, IRoleHandler<Employee>>();
        strategyHandlers.Add("Admin", new RoleAdmin());
        strategyHandlers.Add("Regular", new RoleRegularUser());

Answer 1

I don't think strategy pattern is what you want. I would just refactor your code to extract the ifs to another method.

public List<Phone> GetListOfPhones(Employee emp)
{
   List<Phone> records = new List<Phone>();
   string query = BuildQueryByEmployeePosition(emp);

   var rawItems = QuerySPList(query);

   foreach (SPListItem item in rawItems)
   {
        //business logic 
   }
   return records;
}

private string BuildQueryByEmployeePosition(Employee emp) 
{
    if (emp.Positions.Count == 1 && emp.Positions[0] == "Regular")
        return "";// some querystring    

    if (emp.Positions.Count == 1 && emp.Positions[0] == "OfficeDirector")
        return "";// some querystring    

    if (emp.Positions.Count == 1 && emp.Positions[0] == "Admin")
        return "";// some querystring    

    if (emp.Positions.Count == 2 && emp.Positions.Contains("Regular") && emp.Positions.Contains("OfficeDirector"))
        return "";// some querystring    

    return ""; // some default query
}

Here is a link to a great explanation of strategy pattern (with a real example of a sort algorithm).

It's great to see that you are new to programming and is studying patterns, but beware: using them when they are not needed is an anti-pattern.

Answer 2

I agree with mawalker, you don't need strategy pattern as you have the same behavior for all cases. What do you need is some kind of creational patterns. I would refactor your code using Builder and Chain of Responsibility patterns.

1) Implement base class:

public abstract class EmployeeHandler
{
  private readonly EmployeeHandler _nextHandler;

  protected EmployeeHandler(EmployeeHandler nextHandler)
  {
    _nextHandler = nextHandler;
  }

  public string BuildQuery(Employee emp)
  {
    if (CanHandle(emp))
    {
        return GetQuery(emp);
    }

    if (_nextHandler == null)
    {
        return string.Empty;
    }

    return _nextHandler.BuildQuery(emp);
  }

  protected abstract string GetQuery(Employee emp);

  protected abstract bool CanHandle(Employee emp);
}

2) Define concrete implementations:

public class RegularEmployeeHandler : EmployeeHandler
{
  public RegularEmployeeHandler(EmployeeHandler nextHandler) : base(nextHandler) {
  }

  protected override bool CanHandle(Employee emp)
  {
    return emp.Positions.Count == 1 && emp.Positions[0] == "Regular";
  }

  protected override string GetQuery(Employee emp)
  {
    return "some regular query";
  }
}

public class OfficeDirectorEmployeeHandler : EmployeeHandler
{
  public OfficeDirectorEmployeeHandler(EmployeeHandler nextHandler) : base(nextHandler) {
  }

  protected override bool CanHandle(Employee emp)
  {
    return emp.Positions.Count == 1 && emp.Positions[0] == "OfficeDirector";
  }

  protected override string GetQuery(Employee emp)
  {
    return "some office director query"; 
  }
}

public class AdminEmployeeHandler : EmployeeHandler
{
  public AdminEmployeeHandler(EmployeeHandler nextHandler) : base(nextHandler) {
  }

  protected override bool CanHandle(Employee emp)
  {
    return emp.Positions.Count == 1 && emp.Positions[0] == "Admin";
  }

  protected override string GetQuery(Employee emp)
  {
    return "some admin query"; 
  } 
}

public class RegularAndOfficeDirectorEmployeeHandler : EmployeeHandler
{
  public RegularAndOfficeDirectorEmployeeHandler(EmployeeHandler nextHandler) : base(nextHandler) {
  }

  protected override string GetQuery(Employee emp)
  {
    return "some regular and director query";
  }

  protected override bool CanHandle(Employee emp)
  {
    return emp.Positions.Count == 2 && emp.Positions.Contains("Regular") && emp.Positions.Contains("OfficeDirector");
  }
}

3) And finally you main class will changed like this:

public class YouMainClass
{

  private readonly EmployeeHandler _firstHandler;

  public YouMainClass()
  {
    var regularHandler = new RegularEmployeeHandler(null);
    var officeDirectorHandler = new OfficeDirectorEmployeeHandler(regularHandler);
    var adminHandler = new AdminEmployeeHandler(officeDirectorHandler);
    _firstHandler = new RegularAndOfficeDirectorEmployeeHandler(adminHandler);
  }

  public List<Phone> GetListOfPhones(Employee emp, IQueryBuilder queryBuilder)
  {
    List<Phone> records = new List<Phone>();
    string query = _firstHandler.BuildQuery(emp);

    var rawItems = QuerySPList(query);

    foreach (SPListItem item in rawItems)
    {
      //business logic 
    }

    return records;
  }
}

Answer 3

Firstly, I think you need to be less concerned with the 'internal structure' of the Strategy Pattern and more concerned with its design goal/use.

https://en.wikipedia.org/wiki/Strategy_pattern

the strategy pattern (also known as the policy pattern) is a software design pattern that enables an algorithm's behavior to be selected at runtime. The strategy pattern

• defines a family of algorithms,
• encapsulates each algorithm, and
• makes the algorithms interchangeable within that family.

To me, it looks like you are actually intending to "create" a string, based on some input value.

Therefore you would likely want to use one of the Creational patterns

My suggestion would be likely be Factory Pattern, or possibly Builder pattern. (Both linked in the creational patterns link)

private string BuildQueryByEmployee(Employee emp) {
    // create your builder
    EmployeeQueryBuilder mBuilder = new EmployeeQueryBuilder ();
    // add # of positions
    mBuilder.addPositionCount(emp.Positions.Count);
    // add each position
    for (int i =0 ; i < emp.Positions.Count; i++ ){
        mBuilder.addPosition(emp.Positions[i]);
    }
    // 'build' your query. 
    // and return the query as a string.
    return mBuilder.buildQuery(); 
}

Then inside your EmployeeQueryBuilder class, you handle the complexity of how to build your query based on the # of positions, and what they are.

---

Secondly, Here are some links that might be of use to you.

• https://en.wikipedia.org/wiki/Software_design_pattern (General disc. of Patterns)
• https://en.wikipedia.org/wiki/Design_Patterns (The "Gang of Four" book, essentially the first book on design patterns, and worth reading a few times, and writing each pattern out by hand a few times until you are comfortable with them, this alone will boost your coding ability by 2~3x if you haven't used patterns before, The examples in this book are somewhat out of date, since its motivating factor is writing a Text Editor... but still a 'classic', well worth knowing)
• http://www.cs.wustl.edu/~schmidt/POSA/POSA2/ (One of the first books on patterns that have to deal with Concurrency) (POSA = Pattern Oriented Software Architecture) (might be overkill for now, but 'good to know about' in general)

Answer 4

In my opition it is better you use a kind of STATE PATTERN with COMPOSITE.

Example:

  public abstract class Position
  {
    public abstract List<int> ListOfPhones();
  }

  public class Employer
  {
    public virtual IList<Position> CurrentPositions { get; set; }
  }

  public class Manager : Employer
  {
    public Manager(List<Position> positions)
    {
      this.CurrentPositions = positions;
    }

    public IEnumerable<int> GetNumbers()
    {
      foreach (Position position in this.CurrentPositions)
        foreach (var number in position.ListOfPhones())
          yield return number;

    }
  }

Code above is incomplete, just for you get the ideia.