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

Question

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

Plante

does your null reference exception have a line number associated with it? – Croy

if a.tiebreakerDelta and b.tiebreakerDelta are both 0, then you will return -1 instead of 0 (the line if (value1 == value2) return 0; will never get executed). Don't know if that's the cause of your null problem, but it seems like an inconsistency. – Huntington

Not sure what kind of help you are looking for... Just put breakpoint with condition (a==null || b==null) at the beginning of the method and see why it is called with null... – Forgather

It would really help if you could show a short but complete example of this failing. – Dignity

The whole part at the bottom about Delta could be reviewed to be much much simplier. And I have the feeling that some error could show up in there because you test one value after the other in a specific order. If you want to test which one is closer to 0, you could Math.Abs() them both and just test which one is bigger. However, knowing where a null shows up, would be a lot of help. – Polybius

In your example, we should see as result: Line4, Line1, Line2, Line3? – Polybius

@Polybius - Math.Abs() won't work for me since it's not a sort based on just the distance from zero. It's basically this priority: zero, less than zero (closest first), more than zero (closest first). The null shows up immediately as if the 'a' input is null. My understanding is this is not a null in the list, it's an inconsistency issue. – Plante

Answer 1

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

Saturninasaturnine

Nick - thanks so much, reworking the order as you specified seems to have corrected the issue! If you didn't mind, could you elaborate on the first paragraph a bit more - I would like to really understand why this works vs. the original bug. – Plante

Well, as you can see, the problem with the first code is that when you have two items with same correct property and when both tiebreakerDelta properties are 0 then calling Compare(val1,val2) will return (according to your code) -1, and if you call Compare with those same instances like Compare(val2, val1) you will get -1 again and this is not how it is supposed to work. Imagine you are calling Compare(1,2) and it returns -1 and when you call Compare(2,1) it returns -1 again. Then there is nothing to compare, their order is more important than their value and you shouldn't sort at all. – Saturninasaturnine

Makes total sense to me now, thanks again - once you explained it seems obvious to me. Cheers. – Plante

In adition to this,it seems like sort is doing compare of an item with itself like Compare(a,a) and that should return 0 not -1. That is why you were getting your the exception. – Saturninasaturnine