How to avoid long switch ..need help refactoring

Question

hi guys,

i needed help refactoring the following class,

Following is a class Operation with variety of Operations in switch : i want to avoid the switch statement.I read few articles on using polymorphism and state pattern .but when i refactor the classes i dont get access to many variables,properties
Im confused on whether to use operation as abstract class or to implement an interface.
Just wanted to know which type of refactoring will help in this case
polymorphism or state pattern?
And when to use them?

public class Operation
    {
        public enum OperationType
        {
            add,
            update,
            delete,
            retrieve
        }
        public enum OperationStatus
        {
            Success,
            NotStarted,
            Error,
            Fail,
            InProcess,
            Free
        }

    // raise this event when operation completes
    public delegate void OperationNotifier(Operation operation);
    public event OperationNotifier OperationEvent=null;           

    private OperationStatus _status=OperationStatus.Free;
    public OperationStatus Status
    {
        get { return _status; }
        set { _status = value; }
    }      


    private string _fileName = null;

    public string FileName
    {
        get { return _fileName; }
        set { _fileName = value; }
    }

    private string _opnid = null;

    public string OperationId
    {
        get { return _opnid; }
        set { _opnid = value; }
    }

    private OperationType _type;
    public OperationType Type
    {
        get { return _type; }
        set { _type = value; }
    }

   public void performOperation(OperationType type, string parameters)
    {  

        switch (type)
        {
            case OperationType.add:
                _status = addOperation(parameters);                   
                break;
            case OperationType.update:
               _status = updateOperation(parameters);
                break;
            case OperationType.delete:
                _status = deleteOperation(parameters);
                break;
            case OperationType.retrieve:
                _status = retrieveOperation(parameters);
                break;
            default:
                break;
        }
        if (OperationEvent != null)
            OperationEvent(this);           
       // return true;
    }


    public OperationStatus addOperation(string parameters)
    {           
        DateTime start = DateTime.Now;
         //Do SOMETHING BIG
        TimeSpan timeTaken = DateTime.Now - start;
        System.Diagnostics.Debug.WriteLine("addOperation:-" + _opnid + "-" + _fileName + "--" + timeTaken.Milliseconds); 
        return OperationStatus.Success;

        }
...other operations here....

Calling code is similar to :

  Operation oprnObj;
                Operation.OperationType operationType;

 oprnObj = new Operation();
 oprnObj.FileName = String.Concat("myxmlfile",".xml");
 oprnObj.OperationId = oprnid;
 oprnObj.OperationEvent += new Operation.OperationNotifier(oprnObj_OperationEvent);
 operation="add"; //get From Outside function getOperation()
 operationType = (Operation.OperationType)Enum.Parse(typeof(Operation.OperationType),   operation.ToLower(), true);
 oprnObj.Type = operationType;
 oprnObj.performOperation(operationType, parameters);

References Threads:
Link
Link
..many more.

Answer 1

I think you're on the right track in attempting to use polymorphism here. This should lead you to the Strategy pattern. Refactor the specific operations into subclasses (AddOperation, etc), but also (and this is the step you're probably missing), factor the data (filename, etc), out into a separate object that gets passed to each operation.

I'd create both an Operation interface, as well as an OperationBase that would hold the status.

Answer 2

If you're looking for patterns then I think you should check out the Strategy Pattern and the Chain of Responsiblity Pattern.

http://en.wikipedia.org/wiki/Strategy%5Fpattern

http://en.wikipedia.org/wiki/Chain%5Fof%5Fresponsibility%5Fpattern

Both of these might be useful ways of thinking about this code.

Answer 3

I'd like you to check if this switch case is likely to be spawn at multiple places. If it is just isolated to one place, you may choose to live with this. (The alternatives are more complex).

However if you do need to make the change,
Visit the following link - http://refactoring.com/catalog/index.html
Search for "Replace Type Code", there are 3 possible refactorings that you could use. IMHO the Strategy one is the one that you need.

Answer 4

In C# you can use functions for the strategy, and using an extension method as the function gives you the ability to add operations as and when required:

public enum OperationStatus
{
    Success, NotStarted, Error, Fail, InProcess, Free
}

public class Operation
{
    // raise this event when operation completes
    public delegate void OperationNotifier(Operation operation, OperationStatus status);
    public event OperationNotifier OperationEvent = null;

    public string FileName { get; set; }
    public string OperationId { get; set; }

    public Func<string, OperationStatus> Function { get; set; }

    public void PerformOperation(string parameters)
    {
        OperationStatus status = Function(parameters);

        if (OperationEvent != null)
            OperationEvent(this, status);
    }
}

static class AddOp
{
    public static OperationStatus AddOperation(this Operation op, string parameters)
    {
        DateTime start = DateTime.Now;
        //Do SOMETHING BIG
        TimeSpan timeTaken = DateTime.Now - start;
        System.Diagnostics.Debug.WriteLine("addOperation:-" + op.OperationId + "-" + op.FileName + "--" + timeTaken.Milliseconds);
        return OperationStatus.Success;
    }
}

class Program
{
    static void Main(string[] args)
    {
        Operation oprnObj = new Operation();
        oprnObj.FileName = String.Concat("myxmlfile", ".xml");
        oprnObj.OperationId = "oprnid";
        oprnObj.OperationEvent += new Operation.OperationNotifier(oprnObj_OperationEvent);
        string parameters = "fish";
        oprnObj.Function = oprnObj.AddOperation;
        oprnObj.PerformOperation(parameters);
    }

    public static void oprnObj_OperationEvent(Operation op, OperationStatus status)
    {
        Console.WriteLine("{0} returned {1}", op.Function.Method.Name, status);
    }
}

You could also pass the OperationEvent to the function if you want intermediate status updates.