0

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 */ }

1 Answer 1

1

Downcasts are rarely necessary. When you use downcasts, you are giving up a lot of static type safety. That can sometimes be the right choice, especially for very dynamic systems. But it should be a choice (e.g. because you need to re-configure the system at run time), not just a workaround for inconvenient syntax.

Do you need abstraction?

Before we go any further, you have to ask yourself whether you need any abstraction here at all. Why do you need those abstract classes or interfaces? Why wouldn't it be sufficient to deal with concrete types exclusively? Such abstraction is only necessary when you want to have multiple implementations for Measures and Parameters and so on. Is that given in your case?

Objects vs generics

There are two strategies to abstract over concrete types in the presence of a type system (Cook 2009):

  • Polymorphism or object-oriented techniques declare an interface that the unknown type must satisfy. There can be any number of implementations of an interface. Each object only knows their own true type. Cook calls this the autognosis principle: “An object can only access other object through their public interfaces.” Two consequences:

    1. No downcasting! Therefore, empty interfaces are useless.
    2. Because different objects don't know each others' implementation, this is unsuitable to represent a family of related types.
  • Abstract Data Types (ADTs) define a module boundary where the concrete types have a name on the outside of the boundary, but their implementation is internal to the module. There can be multiple modules that define the same interface and these will be source-compatible, but you would have to recompile to change the module.

  • (The third alternative, performing any checks only at runtime, simply ignores any static typing and won't be discussed any further here.)

Many OOP languages have no straightforward way to implement ADT modules. A single class may be a module as it can mark its implementation private, but a module that provides a family of types is very difficult. To some degree, internal/package-visibility can help, but these languages do not typically support declaring an interface or signature for a module.

Generics are a possible encoding. While they behave a lot like the OOP solution at runtime, they still allow stronger static type checking. In particular, a type variable can name a concrete type without knowing its representation. In your generics solution, the generic Calibration class defines an interface for a module, and the module is instantiated in the Main. The CalibrateSystem is a consumer of the module. However, most of your type constrains are completely superfluous, except as a kind of extra documentation.

A significant drawback of generics-based solutions is that any code using this solution must also be generic, unless it knows the concrete type. OOP approaches don't suffer from this as they “erase” the concrete type, rather than just providing an alias.

Keep internal stuff internal.

In some cases where using OOP techniques would be preferable but this is not possible cleanly due to downcasting, this can indicate that the module boundary was drawn poorly. You have different kinds of measurement systems like A, B, C, … that internally need different types like MeasurementA, ParameterB, …. It makes no sense to combined MeasurementA with ParameterB. So why should users have access to these types? Quite possibly, each complete system should be a object-oriented type that internally uses concrete types for measurements and parameters. Any data flow should then happen internally, rather than going through the public interface.

You are already fairly close to this as your Calibration class effectively represents the complete public interface. In your design the calibration is not part of the measurement system and is a consumer of its interface. This can be avoided if you create subclasses CalibrationA, CalibrationB, … for each system variant.

Choosing the responsibilities differently could also avoid having to expose the concrete types. Let's look at the relationship around the Coefficients type:

Coefficients coefs = CalibrationEngine.ComputeCalibrationCoefficients(...);
CoefficientsPersister.Persist(coefs);

Here the Coefficients and CoefficientsPersister need to match, and the concrete Coefficients type needs to be part of the API. Alternatives:

  • The coefficients can persist themselves, in which case Coefficients can be an OOP interface: coefs.Persist().
  • The coefficients provide an interface that allows any necessary data for persistence to be retrieved, e.g. by returning a representation as a key-value list or as a serializable data structure. Then, the Persister doesn't need to know about the concrete Coefficients implementation.
  • The coefficients can persist themselves, using a Persistence Strategy: coefs.Persist(CoeffPersistenceStrategy). Here, the Strategy doesn't have to know anything about concrete Coefficients and the Coefficients only need to know about the Strategy's interface.

Constructing families of objects.

You mention that the structure of the system depends on configuration. There are two patterns to deal with this without having to expose the concrete types: the Abstract Factory Pattern and the polymorphic Builder Pattern.

An abstract factory is part of the system and knows the concrete types. Internally it can wire up different objects of the same family with each other, while publicly only returning interface types.

The builder pattern is similar: the system's users give the builder commands how to assemble the system, but they do not get access to individual objects. At the end, they receive a fully constructed system through an interface type.

Conclusion

This answer was high on concept, low on concrete advice. Because there is no ”one size fits all“ solution, and the correct choice depends highly on context. Finding a suitable solution is mostly a matter of experience.

Your co-worker is right that the polymorphic solution has advantages despite the downcasts, you are also right that the generic solution has advantages despite its verbosity. I personally would prefer generics because I value static type safety, but I'd change my mind if the context of this code makes such an approach infeasible.

Whichever approach you choose, you should also consider whether your problem can be modeled differently to avoid some of these problems (above, I went into detail on the Coefficients example). By choosing object's responsibilities suitably, you might be able to eliminate some downcasts.

References

  • Cook, William R. (2009): On Understanding Data Abstraction, Revisited.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.