views:

131

answers:

3

I have a class, Deck, that contains a method called Shuffle.

I'm working on refactoring Deck to extend List<Card>, rather than having List<Card> Cards as a property. However, while Cards.OrderBy (a => Guid.NewGuid ()) worked, OrderBy (a => Guid.NewGuid ()) does not:

Error CS0103: The name 'OrderBy' does not exist in the current context (CS0103)

Why does this not work?

+6  A: 

Add this to the front of OrderBy as in

this.OrderBy(a => Guid.NewGuid()); // a random ordering

OrderBy is an extension method on IEnumerable<T> and is not a public method on List<T>. If you type OrderBy with no context the compiler will look for an instance or static method named OrderBy. It is only if you prefix OrderBy with an instance of IEnumerable<T> will the compiler find OrderBy. As Deck : List<Card> and List<Card> : IEnumerable<Card>, using the keyword this (a reference to the current instance) will give the compiler the context it needs to locate the method Enumerable.OrderBy.

It is considered bad practice to inherit from List<T> in a public API. First, List<T> was not designed for inheritance and probably should have been sealed; too late for that now. In general, you should favor composition over inheritance when using framework classes.

Jason
Correction: It's considered bad practice to inherit from `List<T>` *in a public API*. If the inheriting class isn't part of a public API, then it's not a problem.
Kyralessa
+1 Thank you for posting that last paragraph! So often people gloss over that point. I recommend Collection<T> or ObservableCollection<T>.
Josh Einstein
@Kyralessa: Thanks for the correction!
Jason
+3  A: 

OrderBy is an extension method, so it can only be used with a qualifier of type IEnumerable<T>.

You need to write this.OrderBy. (this is a qualifier of type Deck, which indirectly inherits IEnumerable<Card>)

Note that OrderBy is not an in-place sort; if you want to sort the existing instance, call Sort((a, b) => 2 - 2 * rand.Next(0, 1)) where rand is an instance of the Random class.


Note: It is bad practice to inherit List<T>. Instead, you should inherit System.Collections.ObjectModel.Collection<T>.

SLaks
+1  A: 

OrderBy is not a method on List<T> - rather, it's defined as an extension method Enumerable.OrderBy.

Because it's not a method of the class, you need to make the compiler see this. You can do that by calling:

this.OrderBy(a => Guid.NewGuid());

However, I recommend rethinking your approach here. Subclassing List<T> is a bad idea - it's much better to implement IList<T> by encapsulating your List<T> instance. List<T> should be an implementation detail, and not part of the API itself.

Reed Copsey
I think you're right (about re-thinking my approach). I had noticed that I was re-implementing several methods/properties in `List`, such as `Count`. But I don't want to bring in all the methods that I *don't* need along with those that I do. It's pretty simple to wrap the methods that I do need in new methods.
Matthew