Should this (Generic Range Class) Conditional logi

2019-05-15 01:02发布

Shown below is a Generic Range class. The purpose of this is to save a Range and then later when requested specify (boolean) if a given value is in the range.

I have read multiple posts, questions, blogs etc. that say "Replace Conditional with Polymorphism"

My question, is it really worth separating the code out into multiple classes where each class would literally have one line of code. Hopefully the code below will show what I mean.

The class is dependent on two more classes, which are not shown here, but if anyone needs it I can include it later.

namespace Common.Utilities
{
    public class GenericRange<T>
       where T : struct, IComparable<T>
   {
      #region Properties
      public T Min { get; private set; }
      public T Max { get; private set; }
      public GenericRangeType RangeType { get; private set; }
      #endregion

      #region Constructors
      public GenericRange(T min, T max, GenericRangeType rangeType = GenericRangeType.Inclusive)
      {
         // Check Parameters
         Min = min;
         Max = max;
         RangeType = rangeType;
      }
      #endregion

      #region Methods
      #region Private
      private bool IsInclusive(T value)
      {
         return value.IsGreaterThanOrEqualTo(Min) && value.IsLessThanOrEqualTo(Max);
      }

      private bool IsInclusiveMin(T value)
      {
         return value.IsGreaterThanOrEqualTo(Min) && value.IsLessThan(Max);
      }

      private bool IsInclusiveMax(T value)
      {
         return value.IsGreaterThan(Min) && value.IsLessThanOrEqualTo(Max);
      }

      private bool IsExclusive(T value)
      {
         return value.IsGreaterThan(Min) && value.IsLessThan(Max);
      }
      #endregion

      #region Public
      public bool Contains(T value)
      {
         switch (RangeType)
         {
            case GenericRangeType.Inclusive: return IsInclusive(value);
            case GenericRangeType.InclusiveMin: return IsInclusiveMin(value);
            case GenericRangeType.InclusiveMax: return IsInclusiveMax(value);
            case GenericRangeType.Exclusive: return IsExclusive(value);
            default: throw new NotImplementedException();
         }
      }

      public override string ToString()
      {
         return String.Format("Min: {0}, Max: {1}, Type: {2}", Min, Max, RangeType);
      }
      #endregion
      #endregion
    }
}

The only Public methods are: Contain and ToString. If I understand it correctly through Polymorphism I should create a separate concrete class for each of the comparisson types and then make Contain a virtual method.

The main thing I am trying to understand is, what would the benefits/advantages be?

If this is the wrong place for this question, then I am sorry. Let me know and I will move it.

EDIT 1: The additional code to make this complete if anyone needs it:

public static class ComparableExtensions
{
    public static bool IsEqualTo<T>(this T leftHand, T value) where T : IComparable<T>
    {
        return leftHand.CompareTo(value) == 0;
   }

    public static bool IsGreaterThan<T>(this T leftHand, T value) where T : IComparable<T>
    {
        return leftHand.CompareTo(value) > 0;
    }
    public static bool IsGreaterThanOrEqualTo<T>(this T leftHand, T value) where T : IComparable<T>
    {
        return leftHand.CompareTo(value) >= 0;
    }

    public static bool IsLessThan<T>(this T leftHand, T value) where T : IComparable<T>
    {
        return leftHand.CompareTo(value) < 0;
    }
    public static bool IsLessThanOrEqualTo<T>(this T leftHand, T value) where T : IComparable<T>
    {
        return leftHand.CompareTo(value) <= 0;
    }
}   

public enum GenericRangeType
{
    Inclusive,
    Exclusive,
    InclusiveMin,
    InclusiveMax
}

3条回答
Root(大扎)
2楼-- · 2019-05-15 01:19

Breaking it out to different classes lets you extend Contains without changing the existing code. In this case it doesn't make too much sense as you have covered all the bases of contains here, but in other cases that extensibility can be very useful.

查看更多
走好不送
3楼-- · 2019-05-15 01:22

I think what you have is fine, as long as it's not going to need to expand much in the future, and you don't need this to be a public class that can be extended in other assemblies. If you'd like a bit more flexibility, you could either use polymorphism, or make a Func<T, bool> (probably private, since you probably just want to expose the Contains method, and not the fact that it's implemented using a Func) that is set when RangeType is set. Then your Contains method becomes return myFunc(value);.

查看更多
你好瞎i
4楼-- · 2019-05-15 01:24

IMO - you have used generics which is more of a "Template" class rather than purely a base class in the classic OOPS terminology.

What I mean to say is, if you had written classes like:

public class GenericRange{...}

public class IntRange : GenericRange{...}
public class DecimalRange : GenericRange{...}

In this case, it would make sense to really break out the implementation of Contains into the separate sub-types as overridden methods.

But since you are using a code template, you do get the benefits of polymorphic behavior which is dependant on the way you initialize the template class.

So, if you did:

new GenericRange<int>(1, 100, inclusive);
new GenericRange<decimal>(1.0, 100.0, inclusive);

you do already have the polymorphic behavior done, I see this as a great benefit of Generics as it allows you to template such code rather than have specialized sub-classes as shown previously.

查看更多
登录 后发表回答