CODEWITHSUNDEEP
c#visual-studioinfinite-loop
Daniel Mastrorillo
Daniel Mastrorillo

Reputation: 161

List.orderBy is getting stuck in an infinite loop

I have a List<Expense> myList where expense contains 2 fields: decimal Amount and a Status ItemStatus. Status is an enum {Paid, DueSoon, DueToday, Overdue, Unpaid}.

I was trying to sort the list in ascending or descending order however Status.Unpaid needs to always appear last in either ascending or descending order.

Using myList.Sort((x, y) => comparer.Compare(x.ItemStatus, y.ItemStatus)) along with my comparer worked well.

However, after sorting the list by ItemStatus I also wanted to sort the list by Amount. So I decided to use myList = myList.OrderBy(x => x.ItemStatus, comparer).ThenBy(x => x.Amount).ToList() this resulted in an infinite loop somewhere.

The infinite loop was still there when i removed the .ThenBy() method entirely.

I added a static counter to my comparer to try and debug and the OrderBy() method used the comparer 90 times on a list of 30 expenses before entering the infinite loop.

This is my comparer: 

class StatusComparer : IComparer<Status>
{
    public bool IsAscending { get; private set; } = true;

    public StatusComparer(bool isAscending)
    {
        IsAscending = isAscending;
    }

    public int Compare(Status x, Status y)
    {
        if (IsUnpaid(x)) { return IsAscending? 1 : -1; }
        if (IsUnpaid(y)) { return IsAscending ? -1 : 1; }


        return x.CompareTo(y);
    }


    private static bool IsUnpaid(Status status)
    {
        return status.CompareTo(Status.Unpaid) == 0;
    }
}

What am I doing wrong or how can I achieve what I'm trying to do?

Thanks in advance.

Upvotes: 1

Views: 802

Answers (1)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186813

Your implementation of Compare is incorrect

public int Compare(Status x, Status y)
{
    if (IsUnpaid(x)) { return IsAscending? 1 : -1; }
    if (IsUnpaid(y)) { return IsAscending ? -1 : 1; }

    return x.CompareTo(y);
}

Imagine, that we have IsAscending == true, IsUnpaid(x) == true and IsUnpaid(y) == true. In this case

x.Compare(y) == 1 // so x > y
y.Compare(x) == 1 // so y > x

That's why OrderBy may well enter into infinite loop (what is the right order for {x, y} collection if x > y and y > x?). You, probably, want 

public int Compare(Status x, Status y) {
  if (IsUnpaid(x)) { 
    if (!IsUnpaid(y))
      return IsAscending ? -1 : 1; // x is UnPaid, y is Paid
  }
  else if (IsUnpaid(y)) { 
    return IsAscending ? 1 : -1;   // x is Paid, y is UnPaid
  }

  // x and y either both Paid or unPaid
  // If IsAscending should be taken into account, use it as below:
  // return IsAscending ? x.CompareTo(y) : y.CompareTo(x);
  return x.CompareTo(y);
}

Upvotes: 5

Related Questions