1

I am working on a project that generates technical brochures in batch. The 3rd party API that is being used expects POCOs with property names that match field names used in each of the brochure templates. The task I am seeking advice on is with the data source that will be used to populate these POCOs. The data is based on cascading calculations against a Domain Model. I originally entertained the idea of having the Domain be self-calculating, but there are so many calcs required that it seemed obvious to me this needed to be abstracted away.

I created some hypothetical code that closely mimics the design pattern in question, and I am open to any suggestions or reaffirmations as to whether I am approaching this correctly.

My primary concerns:

  1. The constructor for the "AirplaneResultsContext" class, which injects itself to calculation classes which are also publicly exposed properties. Does this create recursive paths?

  2. The possible occurrance of circular references with cascading calculation calls to various properties/methods in various other classes. Note that I am re-using most of the calculation logic (as was requested) but I can propose the need for it to be refactored if need be.

I noticed I used the word "Intricate", which is usually a telltale warning for code that needs to be abstracted out, but I am struggling to see how I can make this better.


The entity being calculated against. Picture this as an aggregate root for a larger domain model.

class Airplane : Entity<int>
{
    public double WingLength { get; set; }
    public double FuselageCircumference { get; set; }
    public double FuselageLength { get; set; }
    public double SeatWidth { get; set; }
    public double SeatDepth { get; set; }
    public double SeatHeight { get; set; }
    public double IsleWidth { get; set; }
    public double LegRoomArea { get; set; }
}

The calculation context. The intent is for this object to be passed along as part of a batch process -- an abstract factory which consumes an interface represented by this context class and generates the POCO objects. Note that I also omitted the interfaces and abstract classes that would be used for different calculation strategies for brevity.

class AirplaneResultsContext
{
    public AirplaneResultsContext(Airplane airplane)
    {
        Airplane = airplane;
        Fuselage = new FuselageCalculator(this);
        Seating = new SeatingCalculator(this);
    }

    public Airplane Airplane { get; private set; }
    public FuselageCalculator Fuselage { get; private set; }
    public SeatingCalculator Seating { get; private set; }

    public double ComputeWidth()
    {
        return Airplane.WingLength*2 + Fuselage.ComputeDiameter();
    }
}

Calculators. In my non-hypothetical project, these methods are private/protected, cached on the first call, and read back by read-only properties, but I only wanted to show a basic representation of some of the calculation complexity.

class SeatingCalculator
{
    private readonly AirplaneResultsContext _context;

    public SeatingCalculator(AirplaneResultsContext context)
    {
        _context = context;
    }

    public int ComputeNumberOfSeats()
    {
        double isleArea = _context.Airplane.IsleWidth* _context.Airplane.FuselageLength;
        double availableArea = (_context.Fuselage.ComputeDiameter()*_context.ComputeWidth()) - isleArea;
        double seatArea = _context.Airplane.SeatDepth * _context.Airplane.SeatWidth + _context.Airplane.LegRoomArea;

        return (int) Math.Floor(availableArea/seatArea);
    }
}

class FuselageCalculator
{
    private readonly AirplaneResultsContext _context;

    public FuselageCalculator(AirplaneResultsContext context)
    {
        _context = context;
    }

    public double ComputeDiameter()
    {
        return Math.PI / _context.Airplane.FuselageCircumference;
    }
}
  • Can you simplify your questions? – Kevin Jul 16 '14 at 0:35
  • I will try to simplify this down when I get back online tomorrow. I'm absorbing the point you made in your answer below about passing the Airplane object in and keeping the shared properties in the context. I can't really add these calcs directly to the domain model, but I might be able to work this into it still by using a visitor/double dispatch approach once I get the issues with my context class straightened out. – bnice7 Jul 16 '14 at 3:13
1

It sounds as though your calculation algorithm consists of the following:

  1. Some large pile of raw data.
  2. A bunch of (sealed or sealable) classes that calculate various results, that are singletons in the context of each specific calculation. E.g. there's one FuselageCalculator, one SeatingCalculator, and so on, for each brochure to be generated.

If that's the case, you can combine the abstract factory pattern with a simple graph coloring algorithm into a calculation manager that will catch any circular references. As long as each calculation stage performs all its calculations in its constructor and demands all preceding results unconditionally, ideally at the beginning of the constructor, the manager can keep track of calculations that are being constructed and throw an exception whenever somebody tries to nest creation of the same type of calculation. Thus each calculation stage becomes an implicit graph node with three possible states: not calculated, calculating, and done calculating.

Here's a quick prototype implementation:

public interface IRawData
{
}

public interface IResult<TRawData> where TRawData : class, IRawData 
{
}

public interface IResultFactory<TRawData> where TRawData : class, IRawData 
{
    Type ResultType { get; }
    IResult<TRawData> CreateResult(CalculationManager<TRawData> manager, TRawData rawData);
}

public class CalculationManagerException : Exception
{
}

public class CircularReferenceException : CalculationManagerException
{
    readonly Type type;
    public CircularReferenceException(Type type)
        : base()
    {
        this.type = type;
    }

    public override string Message
    {
        get
        {
            return GetType().Name + ": Circular reference calculating " + type.Name;
        }
    }
}

public class UnknownResultTypeException : CalculationManagerException
{
}

public class CalculationManager<TRawData> where TRawData : class, IRawData 
{
    class DummyResult : IResult<TRawData>
    {
        internal DummyResult() { }
    }

    readonly TRawData rawData;

    readonly Dictionary<Type, IResult<TRawData>> results = new Dictionary<Type, IResult<TRawData>>();
    readonly Dictionary<Type, IResultFactory<TRawData>> resultFactory;
    readonly DummyResult calculatingFlag = new DummyResult();

    public CalculationManager(TRawData rawData, IEnumerable<IResultFactory<TRawData>> factories)
    {
        this.resultFactory = factories.ToDictionary(x => x.ResultType, x => x );
        this.rawData = rawData;
    }

    public TResult DemandResult<TResult>() where TResult : class, IResult<TRawData>
    {
        IResult<TRawData> iResult;
        if (!results.TryGetValue(typeof(TResult), out iResult))
        {
            IResultFactory<TRawData> factory;
            if (!resultFactory.TryGetValue(typeof(TResult), out factory))
                throw new UnknownResultTypeException();
            Debug.Assert(typeof(TResult).IsSealed);
            results[typeof(TResult)] = calculatingFlag; // Synthetic flag that we are in the middle of calculating results.
            iResult = factory.CreateResult(this, rawData);
            if (iResult.GetType() != typeof(TResult))
            {
                Debug.Assert(iResult.GetType() == typeof(TResult));
                throw new UnknownResultTypeException();
            }
            results[typeof(TResult)] = iResult; 
        }
        else
        {
            if (iResult == calculatingFlag)
                throw new CircularReferenceException(typeof(TResult));
        }
        return (TResult)iResult;
    }
}

(Maybe there's a cleaner way to do the abstract factory pattern in c# 4.5 using covariance? For various reasons I'm stuck at c# 3.0. Maybe Activator.CreateInstance would be prettier, skipping the factory classes entirely?)

Now let's test it. Here are some classes with no circular reference. Result2 depends on Result1 which depends on TestData:

public sealed class TestData : IRawData
{
    public double Field0 { get { return 0; } }
}

public sealed class Result1 : IResult<TestData>
{
    readonly double field1;
    public Result1(CalculationManager<TestData> manager, TestData rawData)
    {
        field1 = rawData.Field0 + 1;
    }

    public double Field1 { get { return field1; } }
}

public sealed class Result1Factory : IResultFactory<TestData>
{
    #region IResultFactory<FirstResult> Members

    public Type ResultType { get { return typeof(Result1); } }

    public IResult<TestData> CreateResult(CalculationManager<TestData> manager, TestData rawData)
    {
        return new Result1(manager, rawData);
    }

    #endregion
}

public sealed class Result2 : IResult<TestData>
{
    readonly double field2;
    public Result2(CalculationManager<TestData> manager, TestData rawData)
    {
        var res1 = manager.DemandResult<Result1>();
        field2 = res1.Field1 + 1;
    }

    public double Field2 { get { return field2; } }
}

public sealed class Result2Factory : IResultFactory<TestData>
{
    #region IResultFactory<FirstResult> Members

    public Type ResultType { get { return typeof(Result2); } }

    public IResult<TestData> CreateResult(CalculationManager<TestData> manager, TestData rawData)
    {
        return new Result2(manager, rawData);
    }

    #endregion
}

Now let's add some classes with circular references (omitting the boring factories):

public sealed class Result3 : IResult<TestData>
{
    readonly double field3;
    public Result3(CalculationManager<TestData> manager, TestData rawData)
    {
        var res1 = manager.DemandResult<Result1>();
        var res4 = manager.DemandResult<Result4>();

        field3 = res1.Field1 + res4.Field4;
    }

    public double Field3 { get { return field3; } }
}

public sealed class Result4 : IResult<TestData>
{
    readonly double field4;
    public Result4(CalculationManager<TestData> manager, TestData rawData)
    {
        var res1 = manager.DemandResult<Result1>();
        var res2 = manager.DemandResult<Result2>();
        var res3 = manager.DemandResult<Result3>();

        if (res1.Field1 > 2)
            field4 = res2.Field2 + res3.Field3;
        else
            field4 = res2.Field2;
    }

    public double Field4 { get { return field4; } }
}

And now, let's test:

public static class TestResult {
    public static void Test()
    {
        try
        {
            TestData data = new TestData();
            CalculationManager<TestData> manager = new CalculationManager<TestData>(data, new IResultFactory<TestData>[] { new Result1Factory(), new Result2Factory(), new Result3Factory(), new Result4Factory() });

            var field2 = manager.DemandResult<Result2>().Field2; // Get two as expected.
            var field4 = manager.DemandResult<Result4>().Field4; // No result -- exception thrown.
        }
        catch (CalculationManagerException e)
        {
            Debug.WriteLine(e.GetType().Name + " " + e.ToString()); // Exception caught: CircularReferenceException: Circular reference calculating Result4
        }
    }
}

Sure enough, Result2 is calculated successfully (and, indirectly, Result1), but calculating Result4 throws the exception.

Is that what you wanted?

Update - I added the coding style constraint that preceding results should be demanded at the beginning of each constructor, and updated my test case to show this style.

BTW, you design may be vulnerable to another type of error: unit mismatch errors. I notice some of your results are linear, some are areas, and perhaps some are dimensionless. It's quite possible that somewhere in the calculation chain a mistaken assumption will get made about the units of a previous calculation. I'd suggest wrapping each numerical result in some sort of unit structure.

  • 1
    Wow, your approach really takes things to the next level! For the unit mismatches, I think I already have an approach to handle that by wrapping each value type into a struct. But this is definitely great and will help me catch any "order of operations" issues. I can probably end up cutting the circular reference checks once I verify none exist, but this will definitely help me find them. – bnice7 Jul 20 '14 at 17:44
1
  1. It can. If you expose your calculator to be called by other calculators is possible to nest them through poor design.
  2. This seems to be the same question as #1

My recommendation for your design would be not to pass the context into your calculators. Any calculations which depend on multiple sub calculations should be properties of the context. Instead just pass in the airplane to the calculator.

My own personal style would be to not to call your calculators 'calculators' but treat them as domain objects (Fuselage, Cabin, Wing etc.) and expose properties, not methods (Diameter, not ComputeDiameter). Properties expose calculations, whereas methods perform operations (calculating and returning values, or changing state)

And FYI there is a stack exchange for code review @ I would recommend moving this to Code Review (https://codereview.stackexchange.com)

  • Thanks Kevin. I had this on code review originally but moved it here after I read that hypothetical examples were not allowed there. Note in my description for the calculator snippet that I omitted the exposed properties and the caching mechanism behind them since I felt comfortable with that aspect, but thanks for the clarification. – bnice7 Jul 16 '14 at 1:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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