Is this a violation of the single responsiblity principle?

Question

I have the following method and interface:

public object ProcessRules(List<IRule> rules)
{
    foreach(IRule rule in rules)
    {
        if(EvaluateExpression(rule.Exp) == true) return rule.Result;
    }

    //Some error handling here for not hitting any rules
}

public interface IRule
{
    Expression Exp;
    Object Result;
    int Precedence;
}

Because rules have a precedence, they should actually never be processed out of order. This leads me with (I think) three solutions:

1. Sort rules before passing them into the evaluator.
2. Change the parameter type to something that enforces a sort order.
3. Sort within the evaluator.

I like option 3 because it always ensures that it is sorted and I like option 1 because it seems more cohesive. And option 2 seems like a good compromise.

Is a scenario like this context specific/subjective, or is there really a best practice to be applied here?

Answer 1

My vote would go for option 3. In an effort to minimize coupling, you want to make sure you don't make too many assumptions about the data sent into a function.

If a different class were to make use of this at a later date, would you assume they know to pass them sorted by precedence?

Answer 2

I think it's more like a violation of the Law of Demeter and encapsulation. The EvaluateExpression looks like it belongs on rules. Consider this:

public object ProcessRules(List<IRule> rules) {
    foreach(IRule rule in rules) {
        return rule.EvaluateExpression();
    }
}

public interface IRule {
    object EvaluateExpression();
}

That way you don't have to expose the internals of rules, like Exp or Result.

And yes, if the behaviour you want is that rules be evaluated in order of precendence, then ensure that they are sorted. The rule's responsibility is to evaluate itself, while it is the caller's decision in what order to evaluate them.

Answer 3

In a scenario like this I'd do something like:

public class RuleProcessor
{   
     public void SortRules(List<IRule> rules){}

     //You could make this an abstract method
     public object ProcessSortedRules(List<IRule> rules)
     {
         foreach(IRule rule in rules)
         {
             if(EvaluateExpression(rule.Exp) == true) return rule.Result;
         }

     //Some error handling here for not hitting any rules

     }

     public object ProcessRules(List<IRule> rules)
     {
          SortRules(rules);
          ProcessSortedRules(rules);
     }

}

You could make that an abstract class or some kind of functionality other classes aggregate.