HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Reusable implementation of IComparable<T>

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
icomparableimplementationreusable

Problem

I often find myself implementing IComparable{T} in different classes for sorting in a very similar fashion:

  • Values meeting criteria X and Y should be sorted to the front



  • Values meeting criteria Z and W should be sorted to the back



  • Values not meeting any special criteria can stay where they are



So, I've created a class that abstracts this comparison style. It works okay, but I feel like there may be a code smell here.

Is there anything conceptually wrong with this class? Is there anything that could be simplified, or made more robust?

```
///
/// Implementation of IComparer that uses a mutable collection of
/// delegates to compare instances.
///
/// Each condition is added with a boolean value indicating
/// whether an instance meeting the condition should be considered
/// "less than" or "greater than" instances not meeting the condition.
/// Conditions are evaluated in the order they are added to the collection.
/// If both instances have the same result for a condition, the condition has no effect.
/// If both instances have the same result for all conditions, the comparison result is 0.
public class DynamicComparer :
IComparer,
IEnumerable, bool>> {

private readonly List, bool>> conditions;
private bool isReadOnly;

public DynamicComparer() {
this.conditions = new List, bool>>();
}

#region Condition collection
public bool IsReadOnly {
get { return isReadOnly; }
set {
if (isReadOnly == true)
throw new InvalidOperationException("Readonly");
isReadOnly = value;
}
}

public void Clear() {
if (isReadOnly == true)
throw new InvalidOperationException("Readonly");
conditions.Clear();
}

public void Add(Func condition, bool prefer) {
if (condition == null) throw new ArgumentNullException("condition");
if (isReadOnly == true)
throw new InvalidOperationException("Readonly");

Solution


  • There is no need to test if a boolean is equals to true, unless it is a nullable boolean (null?).



-
What is a KeyValuePair, bool>? A function T to boolean, and a another boolean is really confusing, mostly the second part which is meant for sort direction. Don't be lazy. If you went ahead and created a dynamic comparer, why left out these two :

public class SortCondition
{
    public SortDirection Direction { get; private set; }
    public Func Condition { get; private set; }

    public SortCondition(SortDirection direction, Func condition)
    {
        this.Direction = direction;
        this.Condition = condition;
    }
}
public enum SortDirection
{
    Start = -1, End = 1
}


-
Prefer and Defer could be combine into a single method, or even fit into Compare.

public int Compare(T x, T y)
{
    foreach (var sortCondition in sortConditions)
    {
        var testX = sortCondition.Condition(x);
        var testY = sortCondition.Condition(y);

        if (testX == testY)
            continue;

        return testX ? +(int)sortCondition.Direction : -(int)sortCondition.Direction;
    }

    return 0;
}


-
Do align your bracket vertically, unless you have a good reason not to.

Full code :

void Main()
{
    var subs = new List
    {
        new SubReport { Name = "LastSubReport" },
        new SubReport { Name = "qwe" },
        new SubReport { Name = "asd" },
        new SubReport { Name = "FirstSubReport" },
        new SubReport { Name = "zxc", IsIssue = true },
        new SubReport { Name = "2ndToLast" },
    };

    subs.Sort(new SubReportComparer());
    subs.Dump();
}

// Define other methods and classes here
public class DynamicComparer : IComparer, IEnumerable>
{
    private readonly List> sortConditions;
    private bool isReadOnly;
    public bool IsReadOnly
    {
        get { return isReadOnly; }
        set
        {
            if (isReadOnly == true)
                throw new InvalidOperationException("Readonly");

            isReadOnly = value;
        }
    }

    public DynamicComparer()
    {
        sortConditions = new List>();
    }
    public void Add(SortDirection direction, Func condition)
    {
        if (condition == null)
            throw new ArgumentNullException("Condition");
        if (isReadOnly == true)
            throw new InvalidOperationException("ReadOnly");

        sortConditions.Add(new SortCondition(direction, condition));
    }
    /// 
    /// Sorts passing item first.
    /// 
    public void MoveToStart(Func condition)
    {
        Add(SortDirection.Start, condition);
    }
    /// 
    /// Sorts failing item first.
    /// 
    public void MoveToEnd(Func condition)
    {
        Add(SortDirection.End, condition);
    }

    public int Compare(T x, T y)
    {
        foreach (var sortCondition in sortConditions)
        {
            var testX = sortCondition.Condition(x);
            var testY = sortCondition.Condition(y);

            if (testX == testY)
                continue;

            return testX ? +(int)sortCondition.Direction : -(int)sortCondition.Direction;
        }

        return 0;
    }

    public IEnumerator> GetEnumerator() { return sortConditions.GetEnumerator(); }
    IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }
}
public class SortCondition
{
    public SortDirection Direction { get; private set; }
    public Func Condition { get; private set; }

    public SortCondition(SortDirection direction, Func condition)
    {
        this.Direction = direction;
        this.Condition = condition;
    }
}
public enum SortDirection
{
    Start = -1, End = 1
}

public class SubReportComparer : DynamicComparer
{
    public SubReportComparer() : base()
    {
        MoveToStart(sub => sub == null);
        MoveToStart(sub => sub.Name == "FirstSubReport");
        MoveToStart(sub => sub.IsIssue);

        MoveToEnd(sub => sub.Name == "LastSubReport");
        MoveToEnd(sub => sub.Name == "2ndToLast");

        IsReadOnly = true;
    }
}

public interface ISubReport
{
    string Name { get; }
    bool IsIssue { get; }
}
public class SubReport : ISubReport
{
    public string Name { get; set; }
    public bool IsIssue { get; set; }
}

Code Snippets

public class SortCondition<T>
{
    public SortDirection Direction { get; private set; }
    public Func<T, bool> Condition { get; private set; }

    public SortCondition(SortDirection direction, Func<T, bool> condition)
    {
        this.Direction = direction;
        this.Condition = condition;
    }
}
public enum SortDirection
{
    Start = -1, End = 1
}
public int Compare(T x, T y)
{
    foreach (var sortCondition in sortConditions)
    {
        var testX = sortCondition.Condition(x);
        var testY = sortCondition.Condition(y);

        if (testX == testY)
            continue;

        return testX ? +(int)sortCondition.Direction : -(int)sortCondition.Direction;
    }

    return 0;
}
void Main()
{
    var subs = new List<SubReport>
    {
        new SubReport { Name = "LastSubReport" },
        new SubReport { Name = "qwe" },
        new SubReport { Name = "asd" },
        new SubReport { Name = "FirstSubReport" },
        new SubReport { Name = "zxc", IsIssue = true },
        new SubReport { Name = "2ndToLast" },
    };

    subs.Sort(new SubReportComparer());
    subs.Dump();
}

// Define other methods and classes here
public class DynamicComparer<T> : IComparer<T>, IEnumerable<SortCondition<T>>
{
    private readonly List<SortCondition<T>> sortConditions;
    private bool isReadOnly;
    public bool IsReadOnly
    {
        get { return isReadOnly; }
        set
        {
            if (isReadOnly == true)
                throw new InvalidOperationException("Readonly");

            isReadOnly = value;
        }
    }

    public DynamicComparer()
    {
        sortConditions = new List<SortCondition<T>>();
    }
    public void Add(SortDirection direction, Func<T, bool> condition)
    {
        if (condition == null)
            throw new ArgumentNullException("Condition");
        if (isReadOnly == true)
            throw new InvalidOperationException("ReadOnly");

        sortConditions.Add(new SortCondition<T>(direction, condition));
    }
    /// <summary>
    /// Sorts passing item first.
    /// </summary>
    public void MoveToStart(Func<T, bool> condition)
    {
        Add(SortDirection.Start, condition);
    }
    /// <summary>
    /// Sorts failing item first.
    /// </summary>
    public void MoveToEnd(Func<T, bool> condition)
    {
        Add(SortDirection.End, condition);
    }

    public int Compare(T x, T y)
    {
        foreach (var sortCondition in sortConditions)
        {
            var testX = sortCondition.Condition(x);
            var testY = sortCondition.Condition(y);

            if (testX == testY)
                continue;

            return testX ? +(int)sortCondition.Direction : -(int)sortCondition.Direction;
        }

        return 0;
    }

    public IEnumerator<SortCondition<T>> GetEnumerator() { return sortConditions.GetEnumerator(); }
    IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); }
}
public class SortCondition<T>
{
    public SortDirection Direction { get; private set; }
    public Func<T, bool> Condition { get; private set; }

    public SortCondition(SortDirection direction, Func<T, bool> condition)
    {
        this.Direction = direction;
        this.Condition = condition;
    }
}
public enum SortDirection
{
    Start = -1, End = 1
}

public class SubReportComparer : DynamicComparer<ISubReport>
{
    public SubReportComparer() : base()
    {
        MoveToStart(sub => sub == null);
        MoveToStart(sub => sub.Name == "FirstSubReport");
        MoveToStart(sub => sub.IsIssue);

        MoveToEnd(sub => sub.Name == "LastSubReport");
        MoveToEnd(sub => sub.Name == "2ndToLast");

        IsReadOnly = true;
    }
}

public interface ISubReport

Context

StackExchange Code Review Q#129299, answer score: 11

Revisions (0)

No revisions yet.