Lou
Lou

Reputation: 4474

Compiling many chunks of code into a single method

I have a legacy method which processes various quantities in real time. There is lots of data, and this method is basically a large if/switch mess which decides how to calculate the target value based on certain rules, and does this for each sample received from each device (and there are many of them). Its signature is something like:

double Process(ITimestampedData data, IProcessingRule rule);

where ISample contains multiple different quantities' values for a single timestamp, while IProcessingRule defines which value to use and how to process it to get the result (which can then be compared to a threshold).

I would like to get rid of all ifs and switches and refactor this into a factory which would create a single processing method for each rule, and then run these methods for input data. Since these rules have various parameters, I would also like to see if there is a way to fully resolve all these branches at compile-time (well, run-time, but I am referring to the point where I invoke the factory method once to "compile" my processing delegate).

So, I have something like this, but much more complex (more mutually-dependent conditions and various rules):

 // this runs on each call
 double result;
 switch (rule.Quantity)
 {
     case QuantityType.Voltage: 
     {
          Vector v;
          if (rule.Type == VectorType.SinglePhase)
          {
             v = data.Vectors[Quantity.Voltage].Phases[rule.Phase];
             if (rule.Phase == PhaseType.Neutral)
             {
                 v = v * 2; // making this up just to make a point
             }
          }
          else if (rule.Type == VectorType.Symmetry)
          {
              v = CalculateSymmetry(data.Vectors);
          }

          if (rule.TargetProperty == PropertyType.Magnitude)
          {
              result = v.Magnitude();
              if (rule.Normalize) 
              {
                   result /= rule.NominalValue;
              }
          }
     }

     // ... this doesn't end so soon

Into something like this:

// this is a factory method which will return a single delegate
// for each rule - and do it only once, at startup
Func<ITimestampedData, double> GetProcessor(IProcessingRule) 
{
    Func<ITimestampedData, Vectors> quantityGetter;
    Func<Vectors, Vector> vectorGetter;
    Func<Vector, double> valueGetter;

    quantityGetter = data => data.Vectors[rule.Quantity];

    if (rule.Type == VectorType.SinglePhase)
    {
        if (rule.Phase == PhaseType.Neutral)
            vectorGetter = vectors => 2 * vectors.Phases[rule.Phase];
        else
            vectorGetter = vectors => vectors.Phases[rule.Phase];
    }
    else if (rule.Type == VectorType.Symmetry)
    {
        vectorGetter = vectors => CalculateSymmetry(vectors);
    }

    if (rule.TargetProperty == PropertyType.Magnitude)
    {
        if (rule.Normalize) 
            valueGetter = v => v.Magnitude() / rule.NominalValue;
        else 
            valueGetter = v => v.Magnitude();
    }
     ...

    // now we just chain all delegates into a single "if-less" call
    return data => valueGetter(vectorGetter(quantityGetter(data)));
 }

But the problem is:

  1. I still have lots of repetition inside my method,
  2. I have switched ifs for multiple delegate invocations and performance doesn't get any better,
  3. although this "chain" is fixed and known at the end of the factory method, I still don't have a single compiled method which would process my input.

So, finally, my question is:

Is there a way to somehow "build" the final compiled method from these various chunks of code inside my factory?

I know I can use something like CSharpCodeProvider, create a huge string and then compile it, but I was hoping for something with better compile time support and type checking.

Upvotes: 0

Views: 71

Answers (2)

Aaron
Aaron

Reputation: 881

Think about having your rules have more functionality inside them. You know what rule you want because you passed it in. But then in your current code you ask the run about itself to determine what calculation you do. I suggest you make the rules more intelligent and ask the rule for the result.

For example the rule that does the most calculations is the SinglePhaseNeutralVoltageMagnitudeNormalizedRule.

class SinglePhaseNeutralVoltageMagnitudeNormalizedRule implements IProcessingRule
{
    double calculate(ITimestampedData data)
    {
        double result;
        Vector v;
        v = data.Vectors[Quantity.Voltage].Phases[Phase];
        v = v * 2; // making this up just to make a point
        result = v.Magnitude();
        result /= NominalValue;
        return result;
    }
}

So the Process method becomes much simpler

result = rule.calculate(data);

A factory class as suggested by @SergeKuharev could be used to build the rules if there is much complexity there. Also, if there is much common code between the rules themselves that could be refactored to a common place.

For example, Normalization could be a rule that simply wraps another rule.

class NormalizeRule IProcessingRule
{
    private IProcessingRule priorRule;
    private double nominalValue;
    public NormalizeRule(IProcessingRule priorRule, double nominalValue)
    {
        priorRule = priorRule;
        nominalValue = nominalValue;
    }
    public double calculate(ITimestampedData data)
    {
        return priorRule.calculate(data)/nominalValue;
    }
}

so given that, and a class SinglePhaseNeutralVoltageMagnitudeRule (as above less the /= nominalValue) a factory could combine the two to make a SinglePhaseNeutralVoltageMagnitudeNrmalizedRule by composition.

Upvotes: 0

Serge Kuharev
Serge Kuharev

Reputation: 1052

Factories

The switch statement is usually a bad smell in code, and your feeling about it are completely right. But factories are perfectly valid place for switch statements. Just don't forget that factory responsibility is to construct objects, so make sure any extra logic is outside of the factory. Also, don't confuse Factories with Factory Methods. First are used when you have a group of polymorphically exchangeable classes and your factory decides which one to use. Also, it helps to break dependencies. At the same time, factory methods are more like static constructors that know about all dependencies of a constructed object. I recommend to be careful about factory methods and prefer proper Factory classes instead. Consider this in terms of SRP - Factory's responsibility is to construct the object while your class has some business responsibility. Whereas you use Factory Method your class gets two responsibilities.

Indentation

There is a good rule I try to follow, called "One indentation level per method". That means, that you can have only more level of indentation, excluding the root one. That is valid and readable piece of code:

function something() {
    doSomething();

    if (isSomethingValid()) {
        doSomethingElse();
    }

    return someResult();
}

Try to follow this rule, by extracting private methods and you will see that code becomes much clearer.

If/Else statements

It is proven that else statement is always optional and you can always refactor your code to not use it. The solution is simple - use early returns. Your methods will become much shorter and way more readable.

Probably my answer is not good enough to solve all your problems, but at least it gives you some ideas to think about.

If you are working with legacy code, I strongly recommend reading "Working Effectively with Legacy Code" book by Michael Feathers and of course "Refactoring" by Martin Fowler.

Upvotes: 1

Related Questions