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! :)
-
\$\begingroup\$ What are you trying to do with that regx? \$\endgroup\$paparazzo– paparazzo2017年08月11日 16:04:14 +00:00Commented Aug 11, 2017 at 16:04
-
\$\begingroup\$ @Paparazzi: which regex? ;P \$\endgroup\$JanDotNet– JanDotNet2017年08月11日 16:08:17 +00:00Commented Aug 11, 2017 at 16:08
-
1\$\begingroup\$ @t3chb0t: usage section added :) \$\endgroup\$JanDotNet– JanDotNet2017年08月11日 16:08:49 +00:00Commented Aug 11, 2017 at 16:08
2 Answers 2
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.
-
\$\begingroup\$ 1) That's true. Actually I would prefer an abstract class, but the generic type
FieldComparer<TFieldValue>
will be created when accessing the staticDefault
property. If it becomes abstract it has to be subclassed by another internal class... Would you prefer that solution? \$\endgroup\$JanDotNet– JanDotNet2017年08月11日 16:55:09 +00:00Commented Aug 11, 2017 at 16:55 -
\$\begingroup\$ @JanDotNet I've added an example and an explanation for my reasoning. \$\endgroup\$t3chb0t– t3chb0t2017年08月11日 17:01:16 +00:00Commented Aug 11, 2017 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– JanDotNet2017年08月11日 17:08:39 +00:00Commented Aug 11, 2017 at 17:08 -
\$\begingroup\$ @JanDotNet if you have numbers then it's fine (and good to know) - you cannot argue with measurements ;-) \$\endgroup\$t3chb0t– t3chb0t2017年08月11日 17:10:52 +00:00Commented Aug 11, 2017 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– t3chb0t2017年08月11日 17:13:05 +00:00Commented Aug 11, 2017 at 17:13
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;
-
\$\begingroup\$ Such obviously... thx! \$\endgroup\$JanDotNet– JanDotNet2017年08月11日 16:10:34 +00:00Commented Aug 11, 2017 at 16:10