patterncsharpMinor
Representing and Parsing an Open or Closed Range
Viewed 0 times
rangeopenclosedparsingandrepresenting
Problem
I would like to represent a numeric range in C#. Either open-ended, such as "up to 35" or "100 on up" or closed-ended, such as "34 to 65". I'd like to represent the open end with NULL. Further, I'd like to parse strings into the range. Strings will look like "-35", "100+", "34-65" or even "-10.5--0.5". I hate that the dash means negative and the range separator, but it is what I'm given. That all being said, I've tested my cases against this code and they work. My questions are: is this the best way to accomplish this? Is it okay to let certain exceptions be thrown for invalid data or should I do more checking on it beforehand? Should this even be called Range? Is Interval better?
```
public struct Range where T : struct, IComparable
{
private static readonly Regex _RemoveSpaces = new Regex(@"\s+", RegexOptions.Compiled);
private static readonly char[] _SplitOnDash = { '-' };
private static readonly TypeConverter _Converter = TypeDescriptor.GetConverter(typeof(T));
private readonly T? _Minimum;
private readonly T? _Maximum;
public Range(T? minimum = null, T? maximum = null)
{
if ((minimum != null) && (maximum != null) && Comparer.Default.Compare(minimum, maximum) > 0)
{
throw new ArgumentException("minimum is greater than maximum.");
}
this._Minimum = minimum;
this._Maximum = maximum;
}
public T? Minimum
{
get
{
return this._Minimum;
}
}
public T? Maximum
{
get
{
return this._Maximum;
}
}
public static Range Parse(string range)
{
range = _RemoveSpaces.Replace(range, string.Empty);
// No maximum.
if (range.EndsWith("+"))
{
return new Range(_Converter.ConvertFromString(range.Substring(0, range.Length - 1)) as T?);
}
// No minimum.
if (range.StartsWith("-") && !range.Substring(2).Contains("-"))
{
```
public struct Range where T : struct, IComparable
{
private static readonly Regex _RemoveSpaces = new Regex(@"\s+", RegexOptions.Compiled);
private static readonly char[] _SplitOnDash = { '-' };
private static readonly TypeConverter _Converter = TypeDescriptor.GetConverter(typeof(T));
private readonly T? _Minimum;
private readonly T? _Maximum;
public Range(T? minimum = null, T? maximum = null)
{
if ((minimum != null) && (maximum != null) && Comparer.Default.Compare(minimum, maximum) > 0)
{
throw new ArgumentException("minimum is greater than maximum.");
}
this._Minimum = minimum;
this._Maximum = maximum;
}
public T? Minimum
{
get
{
return this._Minimum;
}
}
public T? Maximum
{
get
{
return this._Maximum;
}
}
public static Range Parse(string range)
{
range = _RemoveSpaces.Replace(range, string.Empty);
// No maximum.
if (range.EndsWith("+"))
{
return new Range(_Converter.ConvertFromString(range.Substring(0, range.Length - 1)) as T?);
}
// No minimum.
if (range.StartsWith("-") && !range.Substring(2).Contains("-"))
{
Solution
I would check the result of range
against
Returning early will get you some performance here. Doing a
Speaking about
One should not always do what can be done.
IMO this isn't readable at first glance so maybe some good old
If you are using C# 6 you could use the string-interpolation operator
About
__
Inside the
In general you code looks tidy and clean.
range = _RemoveSpaces.Replace(range, string.Empty);against
string.Empty because that is one of the cases where the code reaches this.// No idea what we were given, give a completely open range.
return new Range(null, null);Returning early will get you some performance here. Doing a
null check on range before that call isn't necessary because Regex.Replace() throws a ArgumentNullException for the case that range == null.Speaking about
Regex.Replace I would prefer string.Replace() over Regex.Replace because it seems faster. See: comparing regex replace string replace and stringbuilder replace which has better performanceOne should not always do what can be done.
public override string ToString()
{
return (this._Minimum == null) && (this._Maximum == null)
? string.Empty
: (this._Minimum == null
? string.Format("-{0}", this._Maximum)
: (this._Maximum == null
? string.Format("{0}+", this._Minimum)
: string.Format("{0} - {1}", this._Minimum, this._Maximum)));
}IMO this isn't readable at first glance so maybe some good old
if..else would be better. Having underscore prefixed membervariables and using this is too much.public override string ToString()
{
if ((_Minimum == null) && (_Maximum == null)) { return string.Empty; }
if (_Minimum == null) { return string.Format("-{0}", _Maximum); }
if (_Maximum == null) { return string.Format("{0}+", _Minimum); }
return string.Format("{0} - {1}", _Minimum, _Maximum)));
}If you are using C# 6 you could use the string-interpolation operator
$ instead of string.Format().About
Interval vs Range I would prefer Range because Interval IMO suits only with dates and times but e.g don't suits with int.__
Inside the
Parse() method I would check the found minimum and maximum before calling the constructor. IMO throwing an exception in a constructor should be avoided if possible.In general you code looks tidy and clean.
- You are using good variables- and methodnames.
- You are using braces although they are optional.
- Your code is easy to read.
Code Snippets
range = _RemoveSpaces.Replace(range, string.Empty);// No idea what we were given, give a completely open range.
return new Range<T>(null, null);public override string ToString()
{
return (this._Minimum == null) && (this._Maximum == null)
? string.Empty
: (this._Minimum == null
? string.Format("-{0}", this._Maximum)
: (this._Maximum == null
? string.Format("{0}+", this._Minimum)
: string.Format("{0} - {1}", this._Minimum, this._Maximum)));
}public override string ToString()
{
if ((_Minimum == null) && (_Maximum == null)) { return string.Empty; }
if (_Minimum == null) { return string.Format("-{0}", _Maximum); }
if (_Maximum == null) { return string.Format("{0}+", _Minimum); }
return string.Format("{0} - {1}", _Minimum, _Maximum)));
}Context
StackExchange Code Review Q#109777, answer score: 5
Revisions (0)
No revisions yet.