2
\$\begingroup\$

I'm trying to work around a rule processor so I created the following classes (the interfaces would be straightforward to guess).

public class OrderProcessor : IOrderProcessor
{
    private readonly IRuleEvaluator _ruleEvaluator;

    public OrderProcessor(IRuleEvaluator ruleEvaluator)
    {
        _ruleEvaluator = ruleEvaluator;
    }

    public void Process(Command command)
    {
        var rules = _ruleEvaluator.GetRules(command);
        foreach (var rule in rules)
        {
            rule.Apply(command);
        }
    }
}

public class RuleEvaluator : IRuleEvaluator
{
    private readonly IRuleSet _ruleSet;

    public RuleEvaluator(IRuleSet ruleSet)
    {
        _ruleSet = ruleSet;
    }

    public List<IRule> GetRules(Command command)
    {
        var ruleSet = _ruleSet.GetRules();

        if (!ruleSet.Any())
        {
            return new List<IRule>();
        }

        var rules = from pair in _ruleSet.GetRules()
                    orderby pair.Value descending
                    select pair.Key;

        return rules.Where(r => r.IsApplicable(command)).ToList();
    }
}

public class RuleSet : IRuleSet
{
    private readonly Dictionary<IRule, int> _rules = new Dictionary<IRule, int>();

    public RuleSet(IDependency1 dependency1,
        IDependency2 dependency2)
    {
        _rules.Add(new Rule1(dependency1), 1000);
        _rules.Add(new Rule2(dependency2), 1);
    }

    public Dictionary<IRule, int> GetRules()
    {
        return _rules;
    }
}

I was trying to abstract it in a way that would be easy to add new Rules to the RuleSet, but my issue comes where I see that if the Rules take any dependency I have to add them to RuleSet class. Not sure if this is actually a good practice as I would say it will easily bloat.

\$\endgroup\$
1
  • \$\begingroup\$ you could injecting a collection of dependencies? another way is through get/setter methods, but in short, I don't know any other way of managing dependencies short of injecting them (or setting them via a property or equivalent get/set methods). \$\endgroup\$
    – BenKoshy
    May 31, 2017 at 13:22

2 Answers 2

3
\$\begingroup\$

but my issue comes where I see that if the Rules take any dependency I have to add them to RuleSet class

You managed to find the issue yourself. Your RuleSet class shouldn't be implemented in such way that the rules are always the same (I believe?). What this mean is that when you create a new RuleSet any rules should be able to go into that set.

I don't know what is the purpose of it but it seems your kind of associating custom data to your rules (in this case integers).

So one approach to consider is to include that data in the rules themselves, per instance:

public interface IRule{
    bool IsApplicable(Command command);
    int Data{get;}
}

Another approach to consider is to provide a method on RuleSet that allows you to add a rule to the set, after this have been created:

public class RuleSet : IRuleSet
{
    private readonly Dictionary<IRule, int> _rules = new Dictionary<IRule, int>();

    public RuleSet()
    {
    }

    public Dictionary<IRule, int> GetRules()
    {
        return _rules;
    }

    public void AddRule(IRule rule, int data){
        _rules.Add(rule, data);
    }
}

I don't know what your implementation of IRule here is but you better make sure that it implements GetHashCode and Equals properly because you are using them as a dictionary key. This is true not only the existing IRule classes but all future possible implementations of IRule that can possibly lie on this dictionary.

Another approach as @Adriano suggested, would be to rely on insertion order, or using a ordered collection such as a SortedSet (presuming the rules will not repeat this will do)

\$\endgroup\$
4
  • \$\begingroup\$ Agree, another possibility is to simply drop that data, as far as I can see it's used only for ordering then: 1) use a SortedCollection or 2) Simply rely on insertion order. In this case RuleSet might be simply derived from Collection<Rule>. \$\endgroup\$ May 31, 2017 at 11:13
  • \$\begingroup\$ @AdrianoRepetti Oh yeah I see that OrderBy now. Thanks for pointing that out \$\endgroup\$ May 31, 2017 at 11:19
  • \$\begingroup\$ Yes, it's just for sorting them when returning. A "priority" when applying them. Not sure if I follow what you mean with the SortedSet. The reason for this class is to be able to add the rules I want to be applied in the order of their priorities. So I thought of this to be the point to add them \$\endgroup\$
    – mitomed
    May 31, 2017 at 11:33
  • 2
    \$\begingroup\$ @mitomed What I meant is that you should be able to use a SortedSet and implement an IComparer<IRule> in such a way that data structure will yield all the rules in the order you want. Example: rules have a property called priority, and you would implement an IComparer that would return rule1.Priority - rule2.Priority.. You add a rule with priority 3 and and then a rule with priority 5, but because the data structure is sorted the rule with priority 5 will come up first. \$\endgroup\$ May 31, 2017 at 13:24
2
\$\begingroup\$

You can apply IoC principle to RuleSet as well. Do not create it's dependencies (Rule1, Rule2, etc.) in its constructor. Inject them instead:

public class RuleSet : IRuleSet
{ 
    public RuleSet(IRule[] rules)
    {
        //do stuff
    }
    '''
}

As for those int values, they should probably be exposed as a property of IRule, as Bruno suggested. If you do it, I don't see any reason for RuleSet to exist at its current state. You might as well just inject array of rules directly into your evaluator.

\$\endgroup\$

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.