IComparer sorting by different criterias

Question

I have a ListView and I am trying to sort them where items with a BackColor of red would be first, and then items with a ForeColor of red, and lastly the rest of the items, but all of them should be sorted by name within their group.

I wrote this code but I still get chunks of same items separated all over:

public int Compare (object x, object y)
{
    int CompareResult;
    ListViewItem a = (ListViewItem) x;
    ListViewItem b = (ListViewItem) y;

    if (a.BackColor == Color.FromArgb (200, 0, 0))
    {
        if (b.BackColor == Color.FromArgb(200, 0, 0))
        {
            return a.Text.CompareTo(b.Text);
        }
        else
        {
            return -1;
        }
    }
    else
    {
        if (a.ForeColor == Color.FromArgb(200, 0, 0))
        {
            if (b.ForeColor == Color.FromArgb(200, 0, 0))
            {
                return a.Text.CompareTo(b.Text);
            }
            else
            {
                return -1;
            }
        }
        else
        {
            return 1;
        }
    }
}

score 1 · Answer 1 · answered Jul 12 '12 at 17:47

The problem is that you're missing a couple cases:

If a is non-red, you end up returning 1 every time, whereas you really want to check to see if b has a red foreground or background. If it does, you need to return 1. If it doesn't, you need to compare texts.
If a has a red foreground, but b doesn't, you always return -1, whereas you first need to check if b has a red background. If it does, you need to return 1.

Those are the two that I can see immediately, but to fix this properly, you need to ensure that every possible pair of inputs (red-fore, red-back, and non-red for each of the two inputs) will result in the correct comparison occurring.

Thanks yeah it seems difficult to list all the possibilities. Wish there was another way to specify the sorting priority for an item by itself, not by pairs. — Joan Venge, Jul 12 '12 at 17:48

Sam I am says Reinstate Monica · Answer 2 · 2012-07-12T17:59:13.220

1

You seem to be missing some cases. I'd make sure that all 9 cases are handled. (some might be redundant)

edited Jul 12 '12 at 17:59

answered Jul 12 '12 at 17:47

Sam I am says Reinstate Monica

30,851
12
72
100

He does so have a return value there. If `a.BackColor` is red, but `b.BackColor` isn't, then the method returns -1, which is correct (`a` must come before `b` regardless of text or foreground color.) – dlev Jul 12 '12 at 17:51

A G · Answer 3 · 2012-07-12T18:57:56.070

Refactored a bit:

Not run, but seems correct -

public int Compare(object x, object y)
        {
            ListViewItem a = (ListViewItem)x;
            ListViewItem b = (ListViewItem)y;

            Color red = Color.FromArgb(200, 0, 0);

            int textCompare = a.Text.CompareTo(b.Text);
            bool bothRed = a.BackColor == red && b.BackColor == red;
            bool bothOtherColor = a.BackColor != red && b.BackColor != red;

            return bothRed || bothOtherColor ? textCompare : b.BackColor == red ? 1 : -1;
}

score 1 · Answer 4 · answered Jul 12 '12 at 18:09

You can make this much simpler and easier to read if you first determine which "group" an item is in:

class Comparer : IComparer<ListViewItem> 
{       
    public int Compare (ListViewItem left, ListViewItem right)
    {
        var leftGroup = DetermineGroup(left);
        var rightGroup = DetermineGroup(right);

        if(leftGroup == rightGroup) 
        { 
           return left.Text.CompareTo(right.Text);
        }

        return leftGroup.CompareTo(rightGroup);
    }

    enum Grouping 
    {
        RedBack,
        RedFront,
        Neither
    }

    Grouping DetermineGroup(ListViewItem x) 
    {
        if(x.BackColor == Color.Red) return Grouping.RedBack;
        if(x.ForeColor == Color.Red) return Grouping.RedFront;

        return Grouping.Neither;
    }
}

score 0 · Answer 5 · answered Jul 12 '12 at 17:57

Ok this works:

public int Compare ( object x, object y )
{
    int CompareResult;
    ListViewItem a = ( ListViewItem ) x;
    ListViewItem b = ( ListViewItem ) y;

    Color red = Color.FromArgb(200, 0, 0);

    if (a.BackColor == red)
    {
        if (b.BackColor == red)
        {
            return a.Text.CompareTo(b.Text);
        }
        else
        {
            return -1;
        }
    }
    else
    {
        if (b.BackColor == red)
        {
            return 1;
        }
        else
        {
            if (a.ForeColor == red)
            {
                if (b.ForeColor == red)
                {
                    return a.Text.CompareTo(b.Text);
                }
                else
                {
                    return -1;
                }
            }
            else
            {
                if (b.ForeColor == red)
                {
                    return 1;
                }
                else
                {
                    return a.Text.CompareTo(b.Text);
                }
            }
        }
    }
}

IComparer sorting by different criterias

5 Answers5