jeudi 14 octobre 2021

Advice to get rid of repeating if statement in my class

I recently came across a piece of code at work that has a repeating if-else condition that checks on an enum called OperationType :

public enum OperationType
{ A, B }

Right now the class's job is to run an operation either on device A or on device B, while reading from a SharedDevice and store some values basically for an X,Y plot. We record the characteristics of the SharedDevice in the function of DeviceA or DeviceB. The problem is that we need to iterate over a list of different parameters and send them to the SharedDevice. This list is different for device A and for device B.

Device class:

public class Device
{
    public double CurrentValue { get; }
    public DeviceParameters Parameters { get; set; }
}

And here is the class responsible for executing this operation:

public class MyOperationExecuter
{
    public Device SharedDevice { get; }
    public Device DeviceA { get; }
    public Device DeviceB { get; }

    public List<DeviceParameters> ParametersA { get; }
    public List<DeviceParameters> ParametersB { get; }

    public List<double> XValuesOfA { get; }
    public List<double> YValuesOfA { get; }
    public List<double> XValuesOfB { get; }
    public List<double> YValuesOfB { get; }

    public void DoMyOperation(OperationType operationType)
    {
        List<DeviceParameters> changingDeviceParameters;

        if (operationType == OperationType.A)
        {
            changingDeviceParameters = ParametersA;
        }
        else
        {
            changingDeviceParameters = ParametersB;
        }

        if (operationType == OperationType.A)
        {
            XValuesOfA.Clear();
            YValuesOfA.Clear();
        }
        else
        {
            XValuesOfB.Clear();
            YValuesOfB.Clear();
        }

        foreach (var parameters in changingDeviceParameters)
        {
            // set the device parameters
            SharedDevice.Parameters = parameters;

            // retrieve the device readings and store the values in the correct dataprovider
            if (operationType == OperationType.A)
            {
                XValuesOfA.Add(DeviceA.CurrentValue);
                YValuesOfA.Add(SharedDevice.CurrentValue));
            }
            else
            {
                XValuesOfB.Add(DeviceB.CurrentValue);
                YValuesOfB.Add(SharedDevice.CurrentValue);
            }
        }

        // save updated x,y data
        Save();
    }
}

As you can see there is a repeating if statement which is not very future proof, since we have to check for the enum in every single step. Also we might need to add an C-type device which would result in an ever growing switch statement. We might also need to execute operations on both A and B. How should I refactor this operation so I can keep extending it without this always repeating if-else logic?

Aucun commentaire:

Enregistrer un commentaire