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