I am willing to refactor some code that acts like a controller class executing work embedded in other classes. On one side it looks good as the controller is generic and what changes has been well encapsulated, but on the other side the solution implies lots of downcastings. I have proposed a generics based refactoring but a colleague did not agree on the generics usage that looks to him heavy, more difficult to read and do not feel like what the generics have been designed to solve.
To illustrate my points I have added 2 pieces of code lower. The first one looking like the current solution that I want to refactor and the second one being the refactoring proposition using generics. It is a bit long... but pretty easy to read.
The code is in C# but I did not see the point of restraining the question to the C# community as it could be interesting to OOP programmers in general.
So my questions are:
- Does downcasting this way actually is a smell? and whether it is worth being refactored according to you?
- Does the generics solution makes any sense?
- Do you see another way to refactor it. Maybe some patterns do fit this situation?
The solution to be refactored:
class Program
{
static void Main(string[] args)
{
// Actually the Calibration object creation is in reality encapsulated
// in a factory that can construct different composition of the object,
// depending on some application settings
Calibration calibration = new Calibration()
{
ParameterRetriever = new ParameterRetrieverA(),
Measurer = new MeasurerA(),
CalibrationEngine = new CalibrationEngineA(),
CoefficientsPersister = new CoefficientsPersisterA(),
};
calibration.CalibrateSystem();
}
}
class Calibration
{
public ParameterRetriever ParameterRetriever { get; set; }
public Measurer Measurer { get; set; }
public CalibrationEngine CalibrationEngine { get; set; }
public CoefficientsPersister CoefficientsPersister { get; set; }
public void CalibrateSystem()
{
Parameter parameter = ParameterRetriever.Retrieve();
Measurement measurement = Measurer.Measure();
Coefficients coefs = CalibrationEngine.ComputeCalibrationCoefficients(parameter, measurement);
CoefficientsPersister.Persist(coefs);
}
}
abstract class ParameterRetriever { public abstract Parameter Retrieve(); }
class ParameterRetrieverA : ParameterRetriever
{
public override Parameter Retrieve()
{
ParameterA parameters = new ParameterA();
// Retrieve parameters and fills parameters
return parameters;
}
}
abstract class Measurer { public abstract Measurement Measure(); }
class MeasurerA : Measurer
{
public override Measurement Measure()
{
MeasurementA measurement = new MeasurementA();
// Execute a mesure and fills measurement
return measurement;
}
}
abstract class CalibrationEngine { public abstract Coefficients ComputeCalibrationCoefficients(Parameter parameter, Measurement measurement); }
class CalibrationEngineA : CalibrationEngine
{
public override Coefficients ComputeCalibrationCoefficients(Parameter parameter, Measurement measurement)
{
ParameterA parameterA = parameter as ParameterA;
MeasurementA measurementA = measurement as MeasurementA;
CoefficientsA coefs = new CoefficientsA();
// Compute coefficients
return coefs;
}
}
abstract class CoefficientsPersister { public abstract void Persist(Coefficients coefs); }
class CoefficientsPersisterA : CoefficientsPersister
{
public override void Persist(Coefficients coefs)
{
CoefficientsA coefsA = coefs as CoefficientsA;
// Persist coefsA coefficients
}
}
abstract class Parameter { } // Empty !
class ParameterA : Parameter { /* Some properties */ }
abstract class Measurement { } // Empty !
class MeasurementA : Measurement { /* Some properties */ }
abstract class Coefficients { } // Empty !
class CoefficientsA : Coefficients { /* Some properties */ }
The generic based refactoring proposition:
class Program
{
static void Main(string[] args)
{
// Actually the Calibration object creation is in reality encapsulated
// in a factory that can construct different composition of the object,
// depending on some application settings
Calibration<ParameterA, MeasurementA, CoefficientsA> calibration = new Calibration<ParameterA, MeasurementA, CoefficientsA>()
{
ParameterRetriever = new ParameterRetrieverA(),
Measurer = new MeasurerA(),
CalibrationEngine = new CalibrationEngineA(),
CoefficientsPersister = new CoefficientsPersisterA(),
};
calibration.CalibrateSystem();
}
}
class Calibration<TParameter, TMeasurement, TCoefficients>
where TParameter : Parameter
where TMeasurement : Measurement
where TCoefficients : Coefficients
{
public ParameterRetriever<TParameter> ParameterRetriever { get; set; }
public Measurer<TMeasurement> Measurer { get; set; }
public CalibrationEngine<TParameter, TMeasurement, TCoefficients> CalibrationEngine { get; set; }
public CoefficientsPersister<TCoefficients> CoefficientsPersister { get; set; }
public void CalibrateSystem()
{
TParameter parameter = ParameterRetriever.Retrieve();
TMeasurement measurement = Measurer.Measure();
TCoefficients coefs = CalibrationEngine.ComputeCalibrationCoefficients(parameter, measurement);
CoefficientsPersister.Persist(coefs);
}
}
abstract class ParameterRetriever<TParameter> where TParameter : Parameter { public abstract TParameter Retrieve(); }
class ParameterRetrieverA: ParameterRetriever<ParameterA>
{
public override ParameterA Retrieve()
{
ParameterA parameters = new ParameterA();
// Retrieve parameters and fills parameters
return parameters;
}
}
abstract class Measurer<TMeasurement> where TMeasurement : Measurement { public abstract TMeasurement Measure(); }
class MeasurerA : Measurer<MeasurementA>
{
public override MeasurementA Measure()
{
MeasurementA measurement = new MeasurementA();
// Execute a mesure and fills measurement
return measurement;
}
}
abstract class CalibrationEngine<TParameter, TMeasurement, TCoefficients>
where TParameter : Parameter
where TMeasurement : Measurement
where TCoefficients : Coefficients
{
public abstract TCoefficients ComputeCalibrationCoefficients(TParameter parameter, TMeasurement measurement);
}
class CalibrationEngineA : CalibrationEngine<ParameterA, MeasurementA, CoefficientsA>
{
public override CoefficientsA ComputeCalibrationCoefficients(ParameterA parameter, MeasurementA measurement)
{
CoefficientsA coefs = new CoefficientsA();
// Compute coefficients
return coefs;
}
}
abstract class CoefficientsPersister<TCoefficients> where TCoefficients : Coefficients { public abstract void Persist(TCoefficients coefs); }
class CoefficientsPersisterA : CoefficientsPersister<CoefficientsA>
{
public override void Persist(CoefficientsA coefs)
{
// Persist coefsA coefficients
}
}
abstract class Parameter { } // Empty !
class ParameterA : Parameter { /* Some properties */ }
abstract class Measurement { } // Empty !
class MeasurementA : Measurement { /* Some properties */ }
abstract class Coefficients { } // Empty !
class CoefficientsA : Coefficients { /* Some properties */ }