Custom struct design: Range

Question

I had a need for some inventory management in a recent project. I decided to create a custom struct for managing the concept of a number range. It allows for easy navigation of a collection.

It function similar to .NET's own Enumeration classes, with extension methods, wrapper classes and so forth allowing for dealing with a IRanged<Items> as defined:

 public interface IRanged<out T>
{
 RangeType RangeType { get; }
 T Start { get; }
 T End { get; }
 T Middle { get; }
 int Size { get; }
 T AtPercent(int percent);
 T ValueAtIndex(int index, IRangedEdgeStrategy strategy);
}

The actual core struct class is:

public struct Range : IEquatable<Range>, IRanged<int>, IEnumerable<int>
{
 public Range(int start, int end)
 {
 if (start.Equals(end)) throw new ArgumentException("Cannot create a range of size zero");
 if (start < end)
 {
 Start = start;
 End = end;
 }
 else
 {
 Start = end;
 End = start;
 }
 if (Start < 0)
 {
 RangeType = End <= 0 ? RangeType.Negative : RangeType.NegToPos;
 }
 else
 {
 RangeType = RangeType.Positive;
 }
 }
 public int Start { get; }
 public int End { get; }
 public RangeType RangeType { get; set; }
 public int Middle => (int)Math.Round(((decimal)Start + End) / 2);
 public int Size => Difference(Start, End);
 public bool Contains(Range range)
 {
 return Start <= range.Start && End >= range.End;
 }
 public static int Difference(int a, int b)
 {
 return Math.Abs(a - b) + 1;
 }
 public int ValueAtIndex(int index, IRangedEdgeStrategy strategy)
 {
 return strategy.Handle(this, index);
 }
 public int AtPercent(int percent)
 {
 if (percent <= 0 || percent > 100)
 throw new ArgumentOutOfRangeException("Percentage needs to be a integer value between 1 and 100");
 var p = percent/100f*Size;
 var clampedPercent = (int) Math.Round(p);
 return AsClamped(clampedPercent);
 }
 public int AsClamped(int index)
 {
 return AsClamped(index, Start, End);
 }
 public static int AsClamped(int index, int start, int end)
 {
 return index < start ? start : index > end ? end : index;
 }
 #region IEnumerable
 IEnumerator IEnumerable.GetEnumerator()
 {
 return GetEnumerator();
 }
 public IEnumerator<int> GetEnumerator()
 {
 for (var i = Start; i <= End; i++)
 yield return i;
 }
 #endregion
 #region Equality
 public bool Equals(Range other)
 {
 return Start == other.Start && End == other.End;
 }
 public override bool Equals(object obj)
 {
 if (obj is Range)
 return Equals((Range)obj);
 return false;
 }
 public static bool operator ==(Range a, Range b)
 {
 return a.Equals(b);
 }
 public static bool operator !=(Range a, Range b)
 {
 return !(a == b);
 }
 public override int GetHashCode()
 {
 return Start.GetHashCode() ^ End.GetHashCode();
 }
 #endregion
 public override string ToString() { return $"({Start}>{End})"; }
}

A range list is a wrapper, similar to the ReadOnlyCollection<T> that returns an element as an IRanged, allowing simplified collection navigation.

 public class RangeList<T> : IRanged<T>
{
 private readonly IList<T> _list;
 public RangeList(IList<T> list)
 {
 _list = list;
 }
 private Range R => _list.GetRange();
 public RangeType RangeType => RangeType.Positive;
 public T Start => _list[R.Start];
 public T End => _list[R.End];
 public T Middle => _list[R.Middle];
 public int Size => _list.Count;
 public T AtPercent(int percent)
 {
 return _list[R.AtPercent(percent)];
 }
 public T ValueAtIndex(int index, IRangedEdgeStrategy strategy)
 {
 return _list[R.ValueAtIndex(index, strategy)];
 }
}

A number of helper extensions for easier use:

public static class RangeExtensions
{
 public static Range GetRange<T>(this ICollection<T> x)
 {
 return new Range(0,x.Count-1);
 }
 public static bool HasRange<T>(this ICollection<T> x)
 {
 return x.Count > 0;
 }
 public static bool ContainsRange<T>(this ICollection<T> x, Range r)
 {
 return x.GetRange().Contains(r);
 }
 public static IEnumerable<T> Select<T>(this IList<T> items,Range r)
 {
 if (!items.HasRange())
 return Enumerable.Empty<T>();
 var collectionRange = items.GetRange();
 if(!collectionRange.Contains(r)) throw new ArgumentOutOfRangeException("Collection does not contain inner range");
 T[] elements = new T[r.Size];
 int arrayIndex = 0;
 for (int i = r.Start; i < r.End+1; i++)
 {
 elements[arrayIndex] = items[i];
 arrayIndex++;
 }
 return elements;
 }
 public static IRanged<T> AsRanged<T>(this IList<T> items)
 {
 return new RangeList<T>(items);
 }
}

Any thoughts on naming conventions, choices of extended types, efficiency of collection types etc that might help make this more friendly?

Finally, if anyone is wondering as to the why this becomes more apparent when you nest a range in a range and provide a range value that can be locked to the parent, irrespective of the collection content, for example (green is a RangeValue, blue is the mid-point, grey is the outer range and blue is the inner range.

Here is a visualization

• JanDotNet: What is the methodValueAtIndex for?
• apieceoffruit: @JanDotNet if you have 1->10, what happens when you request index 12? you can pass in a number, and an edge strategy (wrap,clamp etc) and you will get 2 or 10 respectively. used for 'driving' a range, by stepping a selected value up and down a range but handling out of bounds.

Answer 1

Only targeting public struct Range

• If possible a struct should be immutable so you shouldn't let a user of this struct set the RangeType property.

• pre calculate values in the constructor instead of calculating the values if accessed

  public int Middle => (int)Math.Round(((decimal)Start + End) / 2);
  public int Size => Difference(Start, End); 
  

• always use braces {} although they might be optional. Omitting braces can lead to serious bugs, using them will structure your code better and make you code more readable.

• leave your operators some room to breathe.

  var p = percent/100f*Size; 
  

  would be more readable like

  var p = percent / 100f * Size; 
  

• a ternary inside a ternary expression becomes almost unreadable.

  This

  public static int AsClamped(int index, int start, int end)
  {
   return index < start ? start : index > end ? end : index;
  } 
  

  would be more readable like

  public static int AsClamped(int index, int start, int end)
  {
   if (index < start)
   {
   return start;
   }
   return index > end ? end : index;
  } 
  

Answer 2

Overall, it is a really well written piece of code. The code structure and styling looks well-thought-out to me.

Probably the implementation makes absolute sense in context of your use case. However, getting a value of a range by an index doesn't make sense for me because a range isn't aware about indexes.

Same for the property RangeList.RangeType. It doesn't make sense in that context (or at least it is wrong implemented because Start and End return the values from the list that may become negative but RangeList.RangeType always return "Positive").

Actually the 2 implementations of IRanged are not looking like real sub types to me..

I would suggest to

• turn the Range type in a more basic construct
• remove the IRanged inteface from the Range struct
• change the IRanged interface to become more specific (remove RangeType for instance).

That would result in something like: (Note that the method RangeList.FromRange allows you to simply create an IRanged<int> object from an Range:

Range

public struct Range : IEquatable<Range>, IEnumerable<int>
{
 // same as in your question but removed method 'ValueAtIndex'
}

IRanged

public interface IRanged<out T>
{
 T FirstItem { get; }
 T LastItem { get; }
 T MiddleItem { get; }
 int Size { get; }
 T AtPercent(int percent);
 T ValueAtIndex(int index, IRangedEdgeStrategy strategy);
}

RangedList

public class RangeList<T> : IRanged<T>
{
 private readonly IList<T> _list;
 public RangeList(IList<T> list)
 {
 _list = list;
 }
 public static RangeList<int> FromRange(Range range)
 {
 var list = Enumerable.Range(range.Start, range.End).ToList();
 return new RangeList<int>(list);
 }
 private Range IndexRange => _list.GetRange();
 public T FirstItem => _list[IndexRange.Start];
 public T LastItem => _list[IndexRange.End];
 public T MiddleItem => _list[IndexRange.Middle];
 public int Size => _list.Count;
 public T AtPercent(int percent)
 {
 return _list[IndexRange.AtPercent(percent)];
 }
 public T ValueAtIndex(int index, IRangedEdgeStrategy strategy)
 {
 return _list[strategy.Handle(IndexRange, index)];
 }
}

• apieceoffruit: I think the name ValueAtIndex is throwing you. Imagine it was called rangeOfValues.Constrain(someNumber,someEdgeHandler)
• apieceoffruit: forget about the underlying data structure. the interface exposes a navigation tool. so, picture an interface for a gallery viewer application. you can get the first picture, the middle, the last, the picture at 80% in, or throw it effectively any number and say 'get me a picture.' so with 5 pictures, if i pass in the numbers 1,2,3,4,5,6,7,8,9 if the edge handler is clamped i get returned picture: 1,2,3,4,5,5,5,5,5 or wrapped it becomes looped, 1,2,3,4,5,1,2,3,4 etc...
• JanDotNet: @apieceoffruit: To your fist comment: It is more the fact, that the index and the edge handler are passed to the range, and then the edge handler is called with the range and the index... For me, Range seems to be a very basic structur (similar to one of the other primitives). Therfore it looks to me like a method ExecuteOperation(int otherValue, IOperationHandler operationHandler) for the data type Int32. However, if the Range is not indented to be such a basic structure your design is OK.
• JanDotNet: @apieceoffruit: To your second comment. I understand the purpose of the method ValueAtIndex. What I mean is, that the Range object is used in 2 different ways. Of the one part it could be used as IRanged object (that requires the ValueAtIndex method). Of the other part, it is used as a "range type" that provides the methods Contains, Difference, AsClamped. That range type is also used as parameter in the RangeExtensions. ... In my eyes that are 2 different responsibilities that shouldn't be mixed in one type. But that is just my opinion of course;)
• apieceoffruit: I see your point, but I liken it more to a Vector, which in most languages have operations like Magnitude, translate,distance of a line etc, the alternative is the equivalent of a Math static class or a MathHelper. which seems like a more complex solution for common struct arithmetic, ..no the otherhand, Range.Constrain(range,value,edgehandler) might make more sense as a static function instead of a direct member of the contract...hmm....

Answer 3

 return index < start ? start : index > end ? end : index;

I tend to agree that this pattern would be easier to read as an if possibly followed by a ternary. but if you have to do this, consider something like

 return index < start ? start
 : index > end ? end
 : index;

or

 return index < start ? start
 : index > end ? end
 : index;

either of which is much clearer about what goes with what.

Contrast with

 if(index<start){return start;}else if(index>end){return end;}else{return index;}

That's also a confusing way to write the same thing. It's simply too dense. The ternary pares off some of the verbosity, but it leaves the density.

A ternary is dense by nature. Compounding that density is a mistake for readability. I found this much easier to understand once I broke it apart. "Oh, you're constraining index to the interval [start, end]. That makes sense. I can see why you might want to do that."