2

I have a really annoying error whereby my comparer.Compare(x, y) is not getting called. I have an IList that is returning a bunch of entities from the database and then I'm sorting a list of entities inside each entity returned from the database.

ie: in this example each household has many accounts, and i want to sort the sub-list by properties on the accounts entities.

My calling logic is as follow:

      List<Household> households = query.ToList();
        households.Sort(new HouseholdComparer());
        return households;

and my comparer looks like this:

public class HouseholdComparer : IComparer<Household>
{
    public int Compare(Household x, Household y)
    {
        foreach (Account xAccount in x.Accounts)
        {
            foreach (Account yAccount in y.Accounts)
            {
                if (xAccount.StartDate == yAccount.StartDate)
                {
                    if ((xAccount.RevenueT12.HasValue && yAccount.RevenueT12.HasValue)
                        && (xAccount.RevenueT12.Value == yAccount.RevenueT12.Value))
                    {
                        if ((xAccount.AUAAnnual.HasValue && yAccount.AUAAnnual.HasValue)
                            && (xAccount.AUAAnnual.Value == yAccount.AUAAnnual.Value))
                            return 0; // all same whatever result

                        if (!xAccount.AUAAnnual.HasValue || !yAccount.AUAAnnual.HasValue) return 0;
                        if (xAccount.AUAAnnual.Value > yAccount.AUAAnnual.Value) return 1;
                        if (xAccount.AUAAnnual.Value < yAccount.AUAAnnual.Value) return -1;
                    }
                    else
                    {
                        if (!xAccount.RevenueT12.HasValue || !yAccount.RevenueT12.HasValue) return 0;
                        if (xAccount.RevenueT12.Value > yAccount.RevenueT12.Value) return 1;
                        if (xAccount.RevenueT12.Value < yAccount.RevenueT12.Value) return -1;
                    }
                }
                else
                {
                    if (x.StartDate > y.StartDate) return 1;
                    if (x.StartDate < y.StartDate) return -1;
                }
            }
        }
        return 0; // it shouldn't get here
    }

When I run the debugger I get a hit in the constructor but nothing in the compare method, can anyone help?????

Joshy
  • 657
  • 8
  • 20
  • I don't see any calls to Compare(). Did you overload the comparison operators? – Paddyd Sep 23 '13 at 14:51
  • 2
    Did you check whether the list is empty or contains just one element? The `Compare` method wouldn't be called for those cases. – Douglas Sep 23 '13 at 14:51
  • 1
    This would be much easier to diagnose with a short but *complete* example demonstrating the problem. If the list is empty or only has a single element, it doesn't need to call the comparer. – Jon Skeet Sep 23 '13 at 14:52
  • you are not returning the values properly – Anirudha Sep 23 '13 at 14:54
  • Ahh that's my problem!!! the list does contain only one item but 25 sub items, is there a way to call the compare method explicitly when there is one one element in the list ? – Joshy Sep 23 '13 at 14:54
  • 3
    Why do you want to call the compare method when there is only one element in the list? – Justin Sep 23 '13 at 14:56
  • why not sort the sub items directly ? – Anand Sep 23 '13 at 14:56
  • If your `Compare` method has side effects then that's *very bad* and you should fix that. If it doesn't, there'd be no point in calling it on a list with one item. – Servy Sep 23 '13 at 14:57
  • You do know the sort can only sort the items(or item in your case...), it can't sort the sub-items. – gdoron Sep 23 '13 at 15:02
  • Are you trying to sort `Household`s or `Account`s? If just `Household`s, then you don't need `Compare` to be called for a single `Household`. – Tim S. Sep 23 '13 at 15:06

4 Answers4

0

I know three possible reasons:

  1. List has only one element
  2. List is empty
  3. All items in list return different hash codes (GetHashCode) (That is how for example Distinct works)
Oleg Karasik
  • 959
  • 6
  • 17
  • Hash codes have nothing to do with this, and I know of no version of "Distinct" that relies upon hash codes. – Kris Vandermotten Sep 23 '13 at 15:11
  • Basically Distinct of the IEnumerable at checks the inequality of elements by GetHashCode and Equality by Equals. If you do not believe me - here is the answer from the quite authored guy : http://stackoverflow.com/a/5981727/1095750 – Oleg Karasik Sep 23 '13 at 15:19
  • I stand corrected. Enumerable.Distinct uses a hash table internally, and indeed does use GetHashCode() to populate the hash table. That being said, the fact that two items do have the same hash code 1) does not make them equal, and 2) cannot explain why the compare method is not being called. Your point 3 remains incorrect. – Kris Vandermotten Sep 24 '13 at 07:15
  • Yes, you are right I have read my #3 again and found that it should be "different" instead of "same" (edited) – Oleg Karasik Sep 25 '13 at 07:08
  • Unfortunately, this is still incorrect. OP is calling Sort(), not Distinct(). Sort() does not rely on hash codes. – Kris Vandermotten Sep 25 '13 at 12:42
0

From what I read in the comments, it looks like you need an IComparer<Account> implementation, without those foreach loops you have know.

Then, if Accounts is a List<Account>, you could do

  Household household = query.First();
  household.Accounts.Sort(new AccountComparer());
  return household;

Of course, if Accounts isn't a List<Account>, you'd need a different method to sort it, but the general idea is the same.

Kris Vandermotten
  • 10,111
  • 38
  • 49
0

Thanks for all your input guys,

I tried a few of your suggestions and found that the best one was to sort the Accounts within the household directly rather than to try and call a sort on the household that sorts the accounts. This meant I had to change my return type for accounts to be a List and then perform the search as follows:

        //sort the internal Accounts in memory.
        List<Household> households = query.ToList();

        foreach (var household in households)
            household.Accounts.Sort(Compare);

        return households;

the the comparer had to be:

    public static int Compare(Account xAccount, Account yAccount)
    {
        if (xAccount.StartDate == yAccount.StartDate)
        {
            if ((xAccount.RevenueT12.HasValue && yAccount.RevenueT12.HasValue)
                && (xAccount.RevenueT12.Value == yAccount.RevenueT12.Value))
            {
                if ((xAccount.AUAAnnual.HasValue && yAccount.AUAAnnual.HasValue)
                    && (xAccount.AUAAnnual.Value == yAccount.AUAAnnual.Value))
                    return 0; // all same whatever result

                if (!xAccount.AUAAnnual.HasValue || !yAccount.AUAAnnual.HasValue) return 0;
                if (xAccount.AUAAnnual.Value > yAccount.AUAAnnual.Value) return 1;
                if (xAccount.AUAAnnual.Value < yAccount.AUAAnnual.Value) return -1;
            }
            else
            {
                if (!xAccount.RevenueT12.HasValue || !yAccount.RevenueT12.HasValue) return 0;
                if (xAccount.RevenueT12.Value > yAccount.RevenueT12.Value) return 1;
                if (xAccount.RevenueT12.Value < yAccount.RevenueT12.Value) return -1;
            }
        }
        else
        {
            if (xAccount.StartDate > yAccount.StartDate) return 1;
            if (xAccount.StartDate < yAccount.StartDate) return -1;
        }

        return 0; // it shouldn't get here
    }

Thanks for all your help!!!

Joshy
  • 657
  • 8
  • 20
-1

Take a look at this example. The method is declared static and is passed directly to the sort call without creating a seperate class. Another option is to make your Household class implement the IComparer interface. Then you don't have to pass anything to the Sort method, it will use the Compare method that implements IComparer.

Vulcronos
  • 3,428
  • 3
  • 16
  • 24
  • I think you are confusing IComparer with IComparable. There are many scenarios where using comparers is better than being comparable. – Kris Vandermotten Sep 23 '13 at 15:04
  • But there's nothing wrong with creating an `IComparer` if he wants to, and this answer doesn't explain why it isn't being called. – Servy Sep 23 '13 at 15:05
  • @KrisVandermotten I used IComparer since that is what he used in the example. It is also what MSDN lists the Sort funciton as taking. – Vulcronos Sep 23 '13 at 15:16
  • There is absolutely no reason why HouseHold would implement IComparer. It could make sense to have Household implement IComparable though, and in that case the Sort method would not need a comparer. Again, you are confusing the two. – Kris Vandermotten Sep 23 '13 at 15:20