I use the below code to sort List<DataAccessViewModel> list.

Here is the sort order :

1. PriorityScore
2. MName
3. CName
4. FName

It works as expected.

public int Compare(DataAccessViewModel x, DataAccessViewModel y)
{
    if (x == null || y == null)
    {
        return 0;
    }

    return x.CompareTo(y);
}

public int CompareTo(DataAccessViewModel mod)
{
    int retval = (int)(this.PriorityScore?.CompareTo(mod.PriorityScore));
    if(retval != 0)
        return retval;
    else
    {
        retval = (this.MName ?? "zzzzzzzzzzzzz").CompareTo(mod.MName ?? "zzzzzzzzzzzzz");
        if (retval != 0)
            return retval;
        else
        {
            retval = (this.CName ?? "zzzzzzzzzzzzz").CompareTo(this.CName ?? "zzzzzzzzzzzzz");
            if (retval != 0)
                return retval;
            else
                retval = (this.FName ?? "zzzzzzzzzzzzz").CompareTo(this.FName ?? "zzzzzzzzzzzzz");
        }
    }
        
    return retval;
}

But the code looks clunky to me. Is there any better way of doing it or is this it ?

CodePudding user response：

There's a issue in the current code:

public int Compare(DataAccessViewModel x, DataAccessViewModel y) 
{
    if (x == null || y == null)
    {
        return 0;
    }
    ...

Since null equals to any value then all values are equal:

   a == null, null == b => a == b

It's not the rule you want to implement. I suggest something like this

using System.Linq;

...

// static: we don't want "this"
public static int TheCompare(DataAccessViewModel x, DataAccessViewModel y) {
  // Special cases, nulls
  if (ReferenceEquals(x, y)) // if references are shared, then equal
    return 0;
  if (null == x) // let null be smaller than any other value
    return -1;
  if (null == y)
    return 1;
 
  // How we compare strings 
  static int MyCompare(string left, string right) =>
      ReferenceEquals(left, right) ? 0 
    : null == left ? 1    // null is greater than any other string
    : null == right ? -1
    : string.Compare(left, right);

  // Func<int> for lazy computation
  return new Func<int>[] {
      () => x.PriorityScore?.CompareTo(y.PriorityScore) ?? -1,
      () => MyCompare(x.MName, y.MName),
      () => MyCompare(x.CName, y.CName),  
      () => MyCompare(x.FName, y.FName), 
    }
    .Select(func => func())
    .FirstOrDefault(value => value != 0);
}

And then for interface we have

public int CompareTo(DataAccessViewModel other) => TheCompare(this, other);

// I doubt if you want to implement both IComparable<T> and
// IComparer<T> but you can easily do it
public int Compare(DataAccessViewModel x, DataAccessViewModel y) =>
  TheCompare(x, y);

CodePudding user response：

You're almost there: just skip the else statements which follow a return:

public int CompareTo(DataAccessViewModel mod)
{
    // I worry about this line -- if you're sure that PriorityScore is not null,
    // why not use this.PriorityScore.Value to make that clear?
    int retval = (int)(this.PriorityScore?.CompareTo(mod.PriorityScore));
    if (retval != 0)
        return retval;

    retval = (this.MName ?? "zzzzzzzzzzzzz").CompareTo(mod.MName ?? "zzzzzzzzzzzzz");
    if (retval != 0)
        return retval;

    retval = (this.CName ?? "zzzzzzzzzzzzz").CompareTo(this.CName ?? "zzzzzzzzzzzzz");
    if (retval != 0)
        return retval;

    return (this.FName ?? "zzzzzzzzzzzzz").CompareTo(this.FName ?? "zzzzzzzzzzzzz");
}

You can also handle the nullables properly by using Comparer<T>.Default. This will sort null as equal to other null values, but less than any other object.

public int CompareTo(DataAccessViewModel mod)
{
    int retval = Comparer<int?>.Default.Compare(this.PriorityScore, mod.PriorityScore);
    if (retval != 0)
        return retval;

    retval = Comparer<string>.Default.Compare(this.MName, mod.MName);
    if (retval != 0)
        return retval;

    retval = Comparer<string>.Default.Compare(this.CName, mod.CName);
    if (retval != 0)
        return retval;

    return Comparer<string>.Default.Compare(this.FName, mod.FName);
}

CodePudding user response：

Consider leveraging the default behavior of ValueTuples:

IComparer (this should probably not be the same as your class)

public int Compare(DataAccessViewModel x, DataAccessViewModel y)
{
    if (x == null && y == null)
    {
        return 0;
    }

    if (x == null) return -1;
    if (y == null) return 1;

    var thisCompareOrder = (x.PriorityScore, x.MName, x.CName, x.FName);
    var thatCompareOrder = (y.PriorityScore, y.MName, y.CName, y.FName);
    return thisCompareOrder.CompareTo(thatCompareOrder);
}

If you want your class to have these semantics by default, implement IComparable.

public int CompareTo(DataAccessViewModel mod)
{
    if(mod == null) return 1;

    var thisCompareOrder = (this.PriorityScore, this.MName, this.CName, this.FName);
    var thatCompareOrder = (mod.PriorityScore, mod.MName, mod.CName, mod.FName);
    return thisCompareOrder.CompareTo(thatCompareOrder);
}

You might also consider making your original class a record, which has value-based semantics by default.