jtmueller / Collections.Pooled

Fast, low-allocation ports of List, Dictionary, HashSet, Stack, and Queue using ArrayPool and Span.
MIT License
554 stars 48 forks source link

Make PooledStack Indexable and Insertable #25

Open Wixred opened 5 years ago

Wixred commented 5 years ago

Is your feature request related to a problem? Please describe. Although not need for my primary use cases, there are some situations where I'd find it beneficial for a Stack implementation to be indexable and insertable. Unfortunately, most implementations do not support this, therefore I have to use a List and it's Add and RemoveAt and forgo the optimizations done for Pop, Peak, and Push.

Describe the solution you'd like Add an Indexer and implement Insert(int index, T item) for PooledStack

jtmueller commented 5 years ago

@Wixred just to be clear, when indexing in to a stack, would you expect stack[0] to return the same element that would be returned by stack.Peek() (the most-recently-added element) and stack[stack.Count - 1] (or stack[^1]) to return the first element that was added to the stack?

Also, if we're adding an Insert method, should there also be a RemoveAt method and an IndexOf method? Should we just go ahead and implement all of ICollection<T>? Where do we draw the line?

Wixred commented 5 years ago

The thought was the index is a simple wrapper around the underlying array indexer. But you're right, where do you draw the line with this functionality.

jtmueller commented 5 years ago

I'm not necessarily opposed to making the class indexable, I'm just not sure of the semantics. Internally items are stored with the most-recently-added item as the last item in the internal array, and calling Peek/Pop returns the last item (but in Stack semantics, it's the first item). So the actual implementation for the indexer would need to return the value at something like Count - i to give you what I think is probably the most intuitive output.

I'm a little less sanguine about the Insert method. You've got the same "what does the index value actually mean" issue, plus the "where do you draw the line" issue, and I'm not sure if the added functionality would be worth it.