patterncsharpModerate
Reusable implementation of IComparable<T>
Viewed 0 times
icomparableimplementationreusable
Problem
I often find myself implementing
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");
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.