15

Before I even ask, let me get the obvious answer out of the way: The ICollection<T> interface includes a Remove method to remove an arbitrary element, which Queue<T> and Stack<T> can't really support (since they can only remove "end" elements).

OK, I realize that. Actually, my question is not specifically about the Queue<T> or Stack<T> collection types; rather, it's about the design decision of not implementing ICollection<T> for any generic type that is essentially a collection of T values.

Here's what I find odd. Say I have a method that accepts an arbitrary collection of T, and for the purpose of the code I'm writing it would be useful to know the size of the collection. For example (the below code is trivial, for illustration only!):

// Argument validation omitted for brevity.
static IEnumerable<T> FirstHalf<T>(this ICollection<T> source)
{
    int i = 0;
    foreach (T item in source)
    {
        yield return item;
        if ((++i) >= (source.Count / 2))
        {
            break;
        }
    }
}

Now, there's really no reason why this code couldn't operate on a Queue<T> or a Stack<T>, except that those types don't implement ICollection<T>. They do implement ICollection, of course—I'm guessing mainly for the Count property alone—but that leads to weird optimization code like this:

// OK, so to accommodate those bastard Queue<T> and Stack<T> types,
// we will just accept any IEnumerable<T>...
static IEnumerable<T> FirstHalf<T>(this IEnumerable<T> source)
{
    int count = CountQuickly<T>(source);
    /* ... */
}

// Then, assuming we've got a collection type with a Count property,
// we'll use that...
static int CountQuickly<T>(IEnumerable collection)
{
    // Note: I realize this is basically what Enumerable.Count already does
    // (minus the exception); I am just including it for clarity.
    var genericColl = collection as ICollection<T>;
    if (genericColl != null)
    {
        return genericColl.Count;
    }

    var nonGenericColl = collection as ICollection;
    if (nonGenericColl != null)
    {
        return nonGenericColl.Count;
    }

    // ...or else we'll just throw an exception, since this collection
    // can't be counted quickly.
    throw new ArgumentException("Cannot count this collection quickly!");
}

Wouldn't it make more sense to just abandon the ICollection interface completely (I don't mean drop the implementation, of course, as that would be a breaking change; I just mean, stop using it), and simply implement ICollection<T> with explicit implementation for members that don't have a perfect match?

I mean, look at what ICollection<T> offers:

  • Count -- Queue<T> and Stack<T> both have this.
  • IsReadOnly -- Queue<T> and Stack<T> easily could have this.
  • Add -- Queue<T> could implement this explicitly (with Enqueue), as could Stack<T> (with Push).
  • Clear -- Check.
  • Contains -- Check.
  • CopyTo -- Check.
  • GetEnumerator -- Check (duh).
  • Remove -- This is the only one that Queue<T> and Stack<T> don't have a perfect match for.

And here's the real kicker: ICollection<T>.Remove returns a bool; so an explicit implementation for Queue<T> could totally (for example) check if the item to be removed is actually the head element (using Peek), and if so, call Dequeue and return true, otherwise return false. Stack<T> could easily be given a similar implementation with Peek and Pop.

All right, now that I've written about a thousand words on why I think this would be possible, I pose the obvious question: why didn't the designers of Queue<T> and Stack<T> implement this interface? That is, what were the design factors (which I am probably not considering) that led to the decision that this would be the wrong choice? Why was ICollection implemented instead?

I am wondering if, in designing my own types, there are any guiding principles I should consider with respect to interface implementation that I might be overlooking in asking this question. For example, is it just considered bad practice to explicitly implement interfaces that aren't fully supported in general (if so, this would seem to conflict with, e.g., List<T> implementing IList)? Is there a conceptual disconnect between the concept of a queue/stack and what ICollection<T> is meant to represent?

Basically, I sense that there must be a pretty good reason Queue<T> (for example) doesn't implement ICollection<T>, and I don't want to just go blindly forward designing my own types and implementing interfaces in an inappropriate manner without being informed and fully thinking through what I'm doing.

I do apologize for the super-long question.

Dan Tao
  • 125,917
  • 54
  • 300
  • 447
  • 4
    Just another example of MS's poor collection libraries. This problem could have been solved by at the very least separating the interfaces into read-only and read-write versions (with the read-write version inheriting from the read-only version) and also having finer-grained interfaces, focussing only on a specific aspect of a collection rather than include irrelevant properties and methods. So ICollection should not have included Add, Clear or Remove. That should have been left to, e.g., IMutableCollection. The ICollection could be implemented by more classes. – siride Jan 24 '11 at 17:36
  • @siride: Your view of the MS collection libraries mirrors mine. It's irksome that things like indexed properties would have to be defined twice for read-only and read-write versions, but such is life. – supercat Dec 29 '11 at 00:57
  • +1 for the innovation to implement `Remove` alone, that was cool :) And no, this super-long q is well written. – nawfal Jun 08 '14 at 09:40

3 Answers3

6

I can't give the "what was the actual thinking" answer - perhaps one of the designers will give us the real thinkng and I can delete this.

However, putting myself in the mindset of "what if someone came to me to make this decision", I can think of an answer.. let me illustrate with this code:

public void SomeMethod<T>( ICollection<T> collection, T valueA, T valueB)
{

  collection.Add( valueA);
  collection.Add( valueB);

  if( someComplicatedCondition())
  {
    collection.Remove(valueA);
  }
}

(Sure, anyone can create a bad implementation of ICollection<T>, but we expect the framework to set the example). Let's assume Stack/Queue implement as you state in the question. So is the code above right, or does it have edge case bugs because ICollection<T>.Remove() should be checked? If valueA MUST be removed, how do I fix this to work with both Stack and Queue? There are answers, but obviously the code above is wrong in this case - even though it smells reasonable.

So both interpretations are valid, but I'm good with the decision made here - if I had the code above and knew I could be passed a Queue or Stack that I could design around it, but it sure would be an easy bug pit to fall into (everywhere you see ICollection<T>, remember the edge cases for remove!)

Philip Rieck
  • 32,368
  • 11
  • 87
  • 99
  • I think this is an excellent example that makes the reasoning here very concrete. Thanks for helping me get my head around this one. Moving forward, I'll challenge myself to come up with examples where implementing a certain interface "incompletely" will cause highly unexpected behavior—in such cases, I will think long and hard before committing to those interfaces. – Dan Tao Jan 24 '11 at 17:53
1

Ultimately perhaps they are just not ideal fits; if you want a list or collection - use a List<T> or Collection<T> ;p

Re Add - there is an implicit assumption that Add adds to the end of the collection, which is not true of a stack (although it is for a queue). In some ways, it actually confuses me that the enumerator for stack/queue does not dequeue/pop, since I largely expect items in a queue/stack to be fetched once each (and once only).

Maybe also (again, using my enumerator as just an example) people simply couldn't agree on exactly how it should behave in some of those scenarios, and lacking complete agreement, simply not implementing them was a better choice.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • @Marc: No problem; people mix up those two all the time (I know I have). I know what you mean about enumerating over queues/stacks; who ever enumerates a queue without wanting to actually dequeue from it? I guess part of the problem is that the documentation for `IEnumerator(T)` says: "Enumerators can be used to read the data in the collection, but they cannot be used to modify the underlying collection." Kind of put themselves in a corner there. – Dan Tao Jan 23 '11 at 20:40
  • @Dan Tao: The solution would be to not have IStack and IQueue implement IEnumerable, but instead have them provide DequeueAll or PopAll methods which would return an IEnumerable. Note that a PopAll method for a thread-safe IStack could yield different results from manually popping everything from the stack, since a thread-safe PopAll should guarantee that an item pushed around the time of PopAll would either be the topmost item returned or else should not be returned but remain in the stack; a manual sequence of 'pop' operations would have no such guarantee. – supercat Jan 24 '11 at 17:31
  • @supercat: I agree, there are certainly fine ways of accomplishing this behavior without using the `IEnumerable` implementation; that said, I wouldn't agree that `Queue` and `Stack` *shouldn't* implement `IEnumerable`. It just seems like a rare case where you want to enumerate without removing. Personally, I do use extension methods that perform basically the same work as your `DequeueAll` and `PopAll` suggestions. – Dan Tao Jan 24 '11 at 17:49
  • @Dan Tao: There's nothing wrong with Queue and Stack implementing IEnumerable, but having IQueue and IStack do so restricts the design of classes that can implement such interfaces. If one might want the ability to peek at a queue or stack without changing it, one should use an interface that explicitly defines the semantics of doing so, and have that interface inherit from IQueue(Of T) or IStack(Of T). – supercat Jan 24 '11 at 18:22
  • @supercat: OK, so you're talking about *hypothetical* `IQueue` and `IStack` interfaces in a collections library, right? I can agree with your points. AFAIK there are no such interfaces in the BCL, though. – Dan Tao Jan 24 '11 at 18:28
  • @Dan Tao: Hmm... I thought I'd seen implementations of IStack and IQueue, but maybe they were simply interfaces someone else wrote up, rather than MS ones. I'm still not terribly keen on something like a stack implementing IEnumerable by itself, since I would think a method called something like PeekAsEnumerable would have much more natural semantics. – supercat Jan 24 '11 at 23:42
0

Philip has given a good answer (+1). There is another conceptual promise that Remove will break for Stack<T>. The ICollection<T>.Remove is documented as:

Removes the first occurrence of a specific object from the ICollection.

Stack<T> is LIFO, even if Remove was implemented, it will have to remove the last occurrence in case of duplicate objects.

If it doesnt make sense for Stack<T>, it's better avoidable for its equal and opposite cousin.


I would have liked it better if MS:

  • didn't document Remove for ICollection<T> like that. Deleting an equal object somewhere would have made more sense considering how vastly different the internal implementations of various structures are. Forcing the removal of first item looks like influenced from linear searching of simple structures like arrays.

  • had interfaces for queue structures. May be:

    public interface IPeekable<T> : IEnumerable<T> // or IInOut<T> or similar
    {
        int Count { get; }
    
        bool Contains(T item);
        T Peek();
        void Add(T item);
        bool Remove();
        void Clear();
    }
    
nawfal
  • 70,104
  • 56
  • 326
  • 368