2
\$\begingroup\$

I am working on a library that parses query strings and evaluates the result for a given context. The query has the format FIELD OPERATOR VALUE (AND|OR) .... The list of operators is constant, but the field's type can be customized (default primary types are supported right now - other types can be added).

To enable the user of the library to overwrite the behavior of the operators, there is a FieldComparer class that evaluates 2 values for a given operator. It uses the default comparer of the type, but can be extended to provide specialized behavior.

The StringFieldComparer is such a specialized comparer for fields of type string. It adds support for wildcards and regex.

Operators

public enum Operator
{
    Equals,
    LessThen,
    LessThenOrEquals,
    GraterThan,
    GraterThanOrEquals,
    NotEquals,
    Matches,
    NotMatches,
}

FieldComparer

public class FieldComparer<TFieldValue>
{
    protected FieldComparer()
    { }

    public virtual bool Compare(Operator op, TFieldValue left, TFieldValue right)
    {
        var comparer = Comparer<TFieldValue>.Default;
        switch (op)
        {
            case Operator.Equals:
                return comparer.Compare(left, right) == 0;
            case Operator.NotEquals:
                return comparer.Compare(left, right) != 0;
            case Operator.GraterThan:
                return comparer.Compare(left, right) < 0;
            case Operator.GraterThanOrEquals:
                return comparer.Compare(left, right) <= 0;
            case Operator.LessThen:
                return comparer.Compare(left, right) > 0;
            case Operator.LessThenOrEquals:
                return comparer.Compare(left, right) >= 0;
            default:
                throw new NotSupportedException($"Operator '{op}' is not supported for type '{typeof(TFieldValue)}'");
        }
    }

    internal static FieldComparer<TFieldValue> Default { get; } = new FieldComparer<TFieldValue>();
}

StringFieldComparer

public class StringFieldComparer : FieldComparer<string>
{
    private readonly Dictionary<string, Regex> myRegexes = new Dictionary<string, Regex>();

    public override bool Compare(Operator op, string left, string right)
    {
        switch (op)
        {
            case Operator.Matches:
            case Operator.NotMatches:
                var regex = GetRegex(left);
                var match = regex.IsMatch(right);
                return op == Operator.Matches ? match : !match;
            case Operator.Equals:
            case Operator.NotEquals:
                regex = GetWildcardRegex(left);
                match = regex.IsMatch(right);
                return op == Operator.Equals ? match : !match;
            default:
                return base.Compare(op, left, right);
        }
    }

    private Regex GetWildcardRegex(string left)
    {
        var pattern = Regex.Escape(left)
            .Replace(@"\*", ".*")
            .Replace(@"\?", ".");

        pattern = "^" + pattern + "$";

        return GetRegex(pattern);
    }

    private Regex GetRegex(string left)
    {
        Regex regex;
        if (!myRegexes.TryGetValue(left, out regex))
        {
            regex = new Regex(left, RegexOptions.Compiled | RegexOptions.IgnoreCase);
            myRegexes.Add(left, regex);
        }
        return regex;
    }
}

Usage

var comparerStr = new StringFieldComparer();
comparerStr.Compare(Operator.Equals, "Hello*", "Hello World"); // true
comparerStr.Compare(Operator.Matches, "He.lo.*", "Hello World"); // true

var comparerInt = FieldComparer<int>.Default;
comparerInt.Compare(Operator.Equals, 1, 1); // true
comparerInt.Compare(Operator.LessThan, 1, 4); // false

I am not really happy with the method StringFieldComparer.Compare because the op will be checked twice for Equals/NotEquals and Match/NotMatch. Is there a way to realize it with only one check without creating the regex unnecessarily?

As always, any feedback is welcome! :)

\$\endgroup\$
3
  • \$\begingroup\$ What are you trying to do with that regx? \$\endgroup\$ – paparazzo Aug 11 '17 at 16:04
  • \$\begingroup\$ @Paparazzi: which regex? ;P \$\endgroup\$ – JanDotNet Aug 11 '17 at 16:08
  • 1
    \$\begingroup\$ @t3chb0t: usage section added :) \$\endgroup\$ – JanDotNet Aug 11 '17 at 16:08
3
\$\begingroup\$
public class FieldComparer<TFieldValue>
{
  protected FieldComparer()
  { }
}

The right way to prevent a type from being created and allowing only derived types to do it is... to use an abstract class. By making the FieldComparer abstract you won't need to hide the default constructor.

An example implementation could look like this where you have a private class DefaultFieldComparer that implements the FieldComparer:

public abstract class FieldComparer<TFieldValue>
{
    ..

    internal static FieldComparer<TFieldValue> Default { get; } = new DefaultFieldComparer();

    private class DefaultFieldComparer : FieldComparer<TFieldValue> { }
}

I find this is a cleaner solution becasue it informs me that the FieldComparer is a base class and intended be subclassed becasue on its own it has no use but if it has, then it should be instantiable.


private readonly Dictionary<string, Regex> myRegexes = new Dictionary<string, Regex>();

private Regex GetRegex(string left)
{
  Regex regex;
  if (!myRegexes.TryGetValue(left, out regex))
  {
      regex = new Regex(left, RegexOptions.Compiled | RegexOptions.IgnoreCase);
      myRegexes.Add(left, regex);
  }
  return regex;
}

I'm not sure whether it's really necessary to use a dictionary for the regexes. In most cases the static Regex.IsMatch is sufficient and it already does the caching thing:

The static IsMatch(String, String) method is equivalent to constructing a Regex object with the regular expression pattern specified by pattern and calling the IsMatch(String) instance method. This regular expression pattern is cached for rapid retrieval by the regular expression engine.

\$\endgroup\$
8
  • \$\begingroup\$ 1) That's true. Actually I would prefer an abstract class, but the generic type FieldComparer<TFieldValue> will be created when accessing the static Default property. If it becomes abstract it has to be subclassed by another internal class... Would you prefer that solution? \$\endgroup\$ – JanDotNet Aug 11 '17 at 16:55
  • \$\begingroup\$ @JanDotNet I've added an example and an explanation for my reasoning. \$\endgroup\$ – t3chb0t Aug 11 '17 at 17:01
  • 1
    \$\begingroup\$ 2) I did a quick test and the compiled version is ~6-7 times faster than Regex.IsMatch. You are right, in most cases it shouldn't matter, but the comparer may be used very often. Therefore I will keep the caching logic ;) \$\endgroup\$ – JanDotNet Aug 11 '17 at 17:08
  • \$\begingroup\$ @JanDotNet if you have numbers then it's fine (and good to know) - you cannot argue with measurements ;-) \$\endgroup\$ – t3chb0t Aug 11 '17 at 17:10
  • 1
    \$\begingroup\$ @JanDotNet it's a pity that the documentation does not clearly state such behaviour. I find they should not cache any pattern so that you can implement your own caching or there should be an option for it. \$\endgroup\$ – t3chb0t Aug 11 '17 at 17:13
2
\$\begingroup\$

remove some repeated code

var comparer = Comparer<TFieldValue>.Default;
int comp = comparer.Compare(left, right);
switch (op)
{
    case Operator.Equals:
        return comp == 0;
    case Operator.NotEquals:
        return comp != 0;
\$\endgroup\$
1
  • \$\begingroup\$ Such obviously... thx! \$\endgroup\$ – JanDotNet Aug 11 '17 at 16:10

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.