Make a Class it's Own Comparer

Question

I have a class that looks like this

public class Foo
        {
            public int A { get; set; }
            public List<string> Bs { get; set; }
        }

And a comparer that looks like this

public class FooComparer : IComparer<Foo>
        {
            public int Compare(Foo x, Foo y)
            {
                return x.A.CompareTo(y.A);
            }
        }

I wanted to combine these into one thing but was concerned about this not being good practice or just appearing odd. What are your thoughts on this

 public class Foo : IComparer<Foo>
        {
            public int A { get; set; }
            public List<string> Bs { get; set; }

            public int Compare(Foo x, Foo y)
                {
                    return x.A.CompareTo(y.A);
                }
            }

@AdamHouldsworth Example code is [off-topic](http://codereview.stackexchange.com/help/on-topic) for Code Review. — Mast, Jun 29 '15 at 08:50
@Mast Ah, because it is general best practice? Yeah fair enough, still opinionated though. Just didn't want to vote to close without offering some alternatives. — Adam Houldsworth, Jun 29 '15 at 08:51
@Mast Hmm actually I'm not entirely sure I understand the Code Review requirements. To me this is a working piece of code, with a request about whether it is good practice or not, so I'd say it fits. — Adam Houldsworth, Jun 29 '15 at 08:53
@AdamHouldsworth Except that this is hypothetical code. Names like `Foo`, `A`, `Bs`, etc. don't often appear in real code. Hypothetical code is OT for CR. — Nic, Jun 29 '15 at 08:54
@AdamHouldsworth Currently the second off-topic close vote states: `Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example. Questions seeking an explanation of someone else's code are also off-topic.` If you're interested, feel free to join us in [the 2nd monitor](http://chat.stackexchange.com/rooms/8595/the-2nd-monitor) — Mast, Jun 29 '15 at 08:56
@QPaysTaxes Working code doesn't preclude certain names. All SO code should be a working sample. Most code presented will be a contrived view on a much larger code base outside of this site. Fair enough, what is presented isn't self-contained working, but it's close enough to be a targeted review question. — Adam Houldsworth, Jun 29 '15 at 08:57
@AdamHouldsworth I can write a working sample that isn't actual, real code. Just because the code is compilable doesn't mean it's on-topic -- it has to be the actual code, not trimmed down or anything. Read CR's help center. It explains all this quite well. — Nic, Jun 29 '15 at 08:59
@QPaysTaxes Fair enough, didn't realise it was quite so specific in that regard. Very difficult to prove either way, some people obfuscate code as a matter of course because it is coming from private code-bases. — Adam Houldsworth, Jun 29 '15 at 09:01
@QPaysTaxes Crazy... though I'd be hard pressed to detect code like that. I wouldn't just assume `Foo` means close if the code itself is concrete and makes sense to demonstrate a structure (naming and other code review elements wouldn't play a part in all CR questions, surely). — Adam Houldsworth, Jun 29 '15 at 09:05
@AdamHouldsworth See, that falls under asking for design review -- and at CR, we review _code_, not _design_. If you want to provide a hypothetical implementation of your design to show how it would work, fine, but your question is OT. — Nic, Jun 29 '15 at 09:07
@QPaysTaxes This code can be argued either way, but would likely fall again into opinion-based. There are design concerns, but as Xanatos's answer demonstrates, there are also a handful of concrete reasons to take a certain path. I still stand by the fact that it was opinion-based for this site though. Shame to think there isn't a place for this type of question, unless of course it is Programmers, but I haven't read the requirements there either recently. — Adam Houldsworth, Jun 29 '15 at 09:09
@AdamHouldsworth I have a feeling this question would be too broad for [Programmers](http://programmers.stackexchange.com), but hopefully a programmers user will clarify whether or not I'm right about that. — Simon Forsberg, Jun 29 '15 at 11:02
@AdamHouldsworth if it is "primary opinion-based" here then the same applies at Programmers. If this question would have more focus it would likely be an algorithm question, which is on-topic at both sites anyway. Please read: **[What goes on Programmers.SE? A guide for Stack Overflow](http://meta.programmers.stackexchange.com/q/7182/22815)**. — , Jun 29 '15 at 12:36
@Snowman I can easily see something that is off-topic here being on-topic elsewhere so your statement is a bit catch-all. Vice versa, a question can be on-topic in both or off-topic in both. StachExchange sites don't need to be mutually exclusive on their requirements, there is bound to be *some* overlap somewhere. To be honest, I wanted the question closed, but didn't want to completely put the OP out into the cold without any help, I won't be referring to CR or Programmers any time soon because even on something like this I've got different opinions on how to interpret the criteria. — Adam Houldsworth, Jun 29 '15 at 12:49

score 5 · Accepted Answer · answered Jun 29 '15 at 08:45

I don't like very much, because there is the IComparable<T> interface that does exactly this.

Now... being the IComparer<T> a stateless interface, you are free to do it... Nothing bad will really happen... But still I don't like it, because, for example, collections like SortedDictionary<,> "keep" a reference to the comparer, so they would keep a collection to a full object (containing for example a List<string> Bs), wasting memory. And then, what would you pass to the SortedDictionary<,>? An instance of Foo created only to be used as a comparer, or a "full" instance of Foo (containing data)? (having then the problem that its lifetime would then be the same as the SortedDictionary<,>)

Make a Class it's Own Comparer

1 Answers1