3
\$\begingroup\$

I've got a base class GraphUIControl which is inherited by 4 child classes:

  • BubbleGraphUIControl
  • BatchGraphUIControl
  • LineGraphUIControl
  • StackedGraphUIControl

I sometimes put all my GraphUIControl in the same List<GraphUIControl> for some obscure reasons.

I wanted to order my list (first by Graph type, then by name) so I implemented my custom iComparer:

public class GraphUIControlComparer : IComparer<GraphUIControl>
{
    public int Compare(GraphUIControl x, GraphUIControl y)
    {
        if (x.GetType() == y.GetType())
        {
            return String.Compare(x.Parent.Name, y.Parent.Name, StringComparison.Ordinal);
        }
        if (x is BubbleGraphUIControl)
        {
            return -1;
        }
        if (y is BubbleGraphUIControl)
        {
            return 1;
        }
        if (x is BatchGraphUIControl)
        {
            return -1;
        }
        if (y is BatchGraphUIControl)
        {
            return 1;
        }
        if (x is LineGraphUIControl)
        {
            return -1;
        }
        if (y is LineGraphUIControl)
        {
            return 1;
        }
        if (x is StackedGraphUIControl)
        {
            return -1;
        }
        if (y is StackedGraphUIControl)
        {
            return 1;
        }       
        return 0;
    }
}

But it seems very verbose and I'd love to make it shorter (also, as it's my first iComparer, if I made a mistake/nonsense/whatever, don't hesitate to tell me!)

Example:

Let's take A, A1, A2, B & C as :

  • A is a BubblueGraphUIControl
  • A1 is a BubblueGraphUIControl
  • A2 is a BubblueGraphUIControl
  • C is a LineGraphUIControl
  • B is a StackedGraphUIControl

I put it in the list named myList in random mode. When I do myList.Sort(new GraphUIControlComparer()) I want myList to be in the order A, A1, A2, C, B because of the types order following this rule:

  1. BubbleGraphUIControl
  2. BatchGraphUIControl
  3. LineGraphUIControl
  4. StackedGraphUIControl

Then the A, A1, A2 are in alphabetic order.

\$\endgroup\$
  • \$\begingroup\$ I delete my answer, hope someone help you. Probably I don't understood the logic behind this compare. \$\endgroup\$ – mybirthname Apr 3 '15 at 12:56
  • \$\begingroup\$ @mybirthname if you don't understood it means that I explained it bad. I'll edit my post with an example :) \$\endgroup\$ – Thomas Ayoub Apr 3 '15 at 12:57
  • \$\begingroup\$ I see where @mybirthname went wrong now. There are only four types, so saying X is one of those four types will always return 1. Yes. You've got a pickle here, but this solution isn't all that bad. I'm interested to see if anyone has another solution, because i don't see one that would look as clean as what you've already done. \$\endgroup\$ – RubberDuck Apr 3 '15 at 12:59
  • \$\begingroup\$ Now I understood you, thanks for the explanation ! \$\endgroup\$ – mybirthname Apr 3 '15 at 13:08
5
\$\begingroup\$

Under most circumstances, it's a good idea to avoid GetType(). Using it indicates something's wrong with your design. But if you're willing to use it, there's a very interesting way to write this (I won't say it's good, it's just interesting and short):

public class GraphUIControlComparer : IComparer<GraphUIControl>
{
    private int NameCompare(GraphUIControl x, GraphUIControl y)
    {
        return String.Compare(x.Parent.Name, y.Parent.Name, StringComparison.Ordinal);
    }

    private static readonly Dictionary<Type, int> typeLookupDict
      = new Dictionary<Type,int>
      {
         {typeof(BubbleGraphUIControl), 0},
         {typeof(BatchGraphUIControl), 1},
         {typeof(LineGraphUIControl), 2},
         {typeof(StackedGraphUIControl), 3}
      };

    private int TypeLookup(GraphUIControl x)
    {
       return typeLookupDict[x.GetType()];
    }

    public int Compare(GraphUIControl x, GraphUIControl y)
    {
        int tx = TypeLookup(x);
        int ty = TypeLookup(y);
        if (tx == ty)
        {
            return NameCompare(x,y);
        }
        return (tx < ty ? -1 : 1);
    }
}

Also see https://stackoverflow.com/questions/4287537/checking-if-the-object-is-of-same-type

\$\endgroup\$
  • \$\begingroup\$ Interesting solution. I like the lookup, but I think you're right. It has both advantages and disadvantages compared to the original code. ++ \$\endgroup\$ – RubberDuck Apr 3 '15 at 14:57
  • 2
    \$\begingroup\$ This is more or less what I thought when I saw the question. Note you could just do return tx.CompareTo(ty) for the final line. \$\endgroup\$ – Ben Aaronson Apr 3 '15 at 15:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.