How to do suitable GetEnumerator() method for my own Stack<T> implementation?-CodePudding

I have a problem with implementation GetEnumerator() method for my own generic Stack implementation. Below is fragment with code in my Stack class which extends generic IEnumerable, stack field is type: List. How can I create GetEnumerator() method so that the unit tests passed ? Or perhaps I made a mistake with creating Push and Pop methods ? Please help me and explain how and why my solution is not good enough.

        public void Push(T item)
        {
            this.stack.Add(item);
        }

        public T Pop()
        {
            if (this.stack.Count == 0)
            {
                throw new InvalidOperationException("Invalid operation pop, stack is empty.");
            }

            T value = this.stack[this.stack.Count - 1];
            this.stack.RemoveAt(this.stack.Count - 1);
            return value;
        }

        public IEnumerator<T> GetEnumerator()
        {
            if (this.stack == null)
            {
                throw new InvalidOperationException("Stack cannot be changed during iteration.");
            }

            for (int index = this.Count - 1; index >= 0; index--)
            {
                yield return this.stack[index];
            }
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return this.GetEnumerator();
        }

There are nunit tests, which are not passed with this code in my stack implementation.

        [Test]
        [Order(7)]
        public void Iterator_PopFromStack_Throw_InvalidOperationException()
        {
            Assert.Throws<InvalidOperationException>(
                () =>
            {
                foreach (var item in this.stack)
                {
                    this.stack.Pop();
                }
            }, "Stack cannot be changed during iteration.");
        }

        [Test]
        [Order(8)]
        public void Iterator_PushIntoStack_Throw_InvalidOperationException()
        {
            Assert.Throws<InvalidOperationException>(
                () =>
            {
                foreach (var item in this.stack)
                {
                    this.stack.Push(this.value);
                }
            }, "Stack cannot be changed during iteration.");
        }

CodePudding user response：

Please help me and explain how and why my solution is not good enough.

It is not good enough cause it does not monitor for collection change during enumeration.

You can always look at the build into the framework implementation - it maintains _version field which is bumped on every manipulation (i.e. Push or Pop), is "cached" at the start of the enumeration and is checked on every MoveNext invocation.

CodePudding user response：

Well for one thing a stack isn't enumerable, since you can only ever access one element ever. So if you want it to be enumerable, what you're building isn't a stack, and you know that, because the backing collection you chose is List<>, not Stack<>.

With that out of the way, you seem to want the enumeration to throw an exception when the backing collection is changed. There are many ways of doing this, but the work is already done for you in the backing list, so you could simply do:

public IEnumerator<T> GetEnumerator() => stack.GetEnumerator();