Where is the inconsistency in this Icomparer that is causing a null reference?

WillyCornbread picture WillyCornbread · Nov 15, 2012 · Viewed 9.6k times · Source

I'm receiving a null object in my custom IComparer implementation despite no null entries in the collection it is being applied to. My understanding is this can be caused by inconsistencies in the IComparer implementation. I cannot spot where this could be happening in the following code.

For reference the intent is that these are sorted by the 'correct' property first, then if they are the same, it sorts based on the 'tiebreakerDelta' property, which sorts closest to zero without going over.

        public int Compare(IFoolsSortable a, IFoolsSortable b)
    {
        int value1 = a.correct;
        int value2 = b.correct;

        // compare the total correct first
        if (value1 < value2) return 1;
        if (value1 > value2) return -1;

        // total correct is the same, sort on deltas (closest without going over)
        value1 = a.tiebreakerDelta;
        value2 = b.tiebreakerDelta;

        // returning -1 says "put value1 higher in the list than value2" 
        // (higher means closer to the 0 element of the sorted array) 
        if (value1 == 0) return -1; // a zero is higher than anything! 
        if (value2 == 0) return 1; // ditto, for the other arg, if val1 isn't zero 
        if (value1 == value2) return 0; // after that, if they are the same, say so 
        // if both are negative, then the larger one goes higher 
        if (value1 < 0 && value2 < 0) return (value1 > value2) ? -1 : 1;
        // if only one is negative, it goes higher 
        if (value1 < 0) return -1;
        if (value2 < 0) return 1;
        // finally, if both are postitive, lower one goes higher 
        return (value1 > value2) ? 1 : -1;
    }

Thanks for any help you might be able to offer!

EDIT: I am certain this is not a true null reference, it is being caused by some inconsistency. Also, occasionally this error text is displayed confirming -

Unable to sort because the IComparer.Compare() method returns inconsistent results. Either a value does not compare equal to itself, or one value repeatedly compared to another value yields different results. x: '',  x's type: 'ResultsLineViewModel',

Breakpoints do not help me with this unfortunately.

EDIT: Here is a short example where ResultsLineViewModel implements the IFoolsSortable interface:

List<ResultsLineViewModel> ls = new List<ResultsLineViewModel>();
        ResultsLineViewModel line1 = new ResultsLineViewModel();
        line1.correct = 10;
        line1.tiebreakerDelta = 0;
        ls.Add(line1);

        ResultsLineViewModel line2 = new ResultsLineViewModel();
        line2.correct = 10;
        line2.tiebreakerDelta = 2;
        ls.Add(line2);

        ResultsLineViewModel line3 = new ResultsLineViewModel();
        line3.correct = 10;
        line3.tiebreakerDelta = -3;
        ls.Add(line3);

        ResultsLineViewModel line4 = new ResultsLineViewModel();
        line4.correct = 9;
        line4.tiebreakerDelta = 0;
        ls.Add(line4);

        ls.Sort(new FoolsSort());

The correct sort for this would be: Line1, line3, line2, line4

Answer

Nikola Davidovic picture Nikola Davidovic · Nov 15, 2012

If a is greater than b then Compare(a,b) should return 1 and Compare(b,a) should return -1. And if you have a.correct = b.correct, and both a.tiebreakerDelta = 0 and b.tiebreakerDelta = 0 then this won't be consistent to the Compare method because you want to retain the order of the operation.

As far as I can see, you should first do this

if (value1 == value2) return 0; // after that, if they are the same, say so

and then this:

if (value1 == 0) return -1; // a zero is higher than anything! 
if (value2 == 0) return 1; // ditto, for the other arg, if val1 isn't zero 

Also note that your logic is reversed, if first is grater than second, you should return 1, not -1. Check for instance this link