2
\$\begingroup\$

I am designing a query builder, where the user can just add conditions or groups, and each group can, in turn, have conditions in it. I came up wit the below class design. Can this design be improved?

public enum RuleType
{
    Group,
    Condition
}
public enum Operator
{
    And,
    Or
}
public enum ConditonValues
{
    GreaterThan = 1,
    EqualTo = 2,
    LessThan = 3
}
public abstract class RuleParent
{
    public Guid Id { get; set; }
    public int Order { get; set; }
    public RuleType RuleType { get; set; }
}
public class RuleSet : RuleParent
{
    public ConditonValues Condition { get; set; }
    public string Field { get; set; }
    public object Data { get; set; }
}
public class MyGroup : RuleParent
{
    public Operator Operator { get; set; }
    public List<RuleParent> Rules { get; set; }
    public MyGroup()
    {
        Rules = new List<RuleParent>();
    }
}

Example usage:

var groupM = new MyGroup();
groupM.Rules.AddRange(new List<RuleParent>
{
     new RuleSet() { Condition =  ConditonValues.EqualTo, Field="name", Data ="reza" , RuleType = RuleType.Condition },
     new MyGroup() {
         Rules = new List<RuleParent>()
         {
             new RuleSet() { Condition =  ConditonValues.EqualTo, Field="category", Data ="Tools" , RuleType = RuleType.Condition  },
             new RuleSet() { Condition =  ConditonValues.GreaterThan, Field="price", Data ="100" , RuleType = RuleType.Condition  }
         }
         , RuleType = RuleType.Group
     }
});
\$\endgroup\$
1
\$\begingroup\$

welcome to code review.

Your organization is clean, I mostly just see a few holes in your logic:

  • Combining groups - A group applies the same operator (and, or) to all rules in it. e.g. rule1 AND rule2 AND rule3. However, since groups cannot contain other groups, you cannot combine operations, e.g. rule1 AND (rule2 OR rule3)
  • Missing Operator/condition - you should add NOT to your enum(s) of operators and/or conditions
  • Rule Type - I'm not sure what the purpose of this enum is. If you think that you need it, you could leave it is as, or alternatively make an IRuleParent interface, then make ConditionRuleParent and GroupRuleParent subclasses (this would be the more C#-ish way to model it). By doing it the swcond way, you can declare the type (condition/group) in the class name when you instantiate it, rather than tacking it on as a mandatory argument.
  • Clunky syntax - it's object oriented, but very verbose to use and mentally parse, which leads us to...
  • Greenspun's tenth rule - Any sufficiently complicated C or Fortran program contains an ad-hoc, informally-specified, bug-ridden, slow implementation of half of Common Lisp. (or LINQ in this case). While there are benefits to rolling your own implementation, you are basically recreating a subset of LINQ (the fluent syntax style). If you want something like LINQ, just use LINQ. If you are rolling your own, you might want to spend a little bit of time learning how it works (from a user point of view, at a minimum) and then steal ideas from it as you write your own version.
\$\endgroup\$

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.