Tomeamis
Tomeamis

Reputation: 477

C# List.Sort doesn't sort

I have a List<T> I want to sort, so I used List<T>.Sort(Comparison<T>). My code didn't work as expected and after some debugging I found that while the order of the items contained within did indeed change, it didn't become sorted.
The code is here:

System.Comparison<Intersection> comp=(Intersection one, Intersection other)=>{//Sort sorts from lowest to highest
    if(one.index>other.index){
        return 1;
    }
    else if(one.index<other.index){
        return -1;
    }
    else if((one.point-one.node.position).sqrMagnitude>(other.point-other.node.position).sqrMagnitude){
        return 1;
    }
    else{
        return -1;
    }
};
intersections.Sort(comp);

Trouble is, after the sort the items can be found in order such that the third item has index 7 while the fourth has 6. I thought that there might be something wrong with the comparison lambda, but I added a code which used the same function to compare sequential items, but it behaved correctly and sometimes returned 1, so the problem is clearly elsewhere.
The afterward comparison:

for(int he=1; he<intersections.Count; he++){
    Debug.Log(comp(intersections[he-1], intersections[he]));
}

Is there something I'm missing or is my List<T>.Sort implementation faulty and I should just make my own sorting method?

The structure looks like this:

class Intersection{
    public PolyNode node;
    public int index;
    public Polygon poly;
    public Intersection sister;
    public bool is_out;
    public sbyte wallnum;
    public Vector2 point;
    public int list_index;
}

Upvotes: 2

Views: 4229

Answers (2)

Eric Lippert
Eric Lippert

Reputation: 659956

To expand on 500 Internal Server Error's (correct) answer, Quicksort requires a well-behaved comparison function. You are required to provide a comparison that:

  • is reflexive: an item must compare equal to itself
  • is antisymmetric in inequality: if A is greater than B then B must also be smaller than A
  • is symmetric in equality: if A is equal to B then B must also be equal to A
  • is transitive: if A is equal to B and B is equal to C then A must be equal to C. If A is greater than B and B is greater than C, then A must be greater than C. And so on

In short, a total ordering relation must be supplied. Your comparison algorithm violates many of these requirements. Any time you fail to provide a total ordering relation, bad things can happen. The algorithm can crash, it can go into infinite loops, or it can return an unsorted list.

For a longer discussion, see my four-part series on common ways that I've seen incorrect comparison algorithms written:

http://ericlippert.com/2011/01/20/bad-comparisons-part-one/

Upvotes: 14

As others also noted, your comparison function never return a result of zero (equal), but List.Sort relies on the comparison function to return equal when an item is compared to itself.

Upvotes: 6

Related Questions