See any problems with this C# implementation of a stack?

Question

I wrote this quickly under interview conditions, I wanted to post it to the community to possibly see if there was a better/faster/cleaner way to go about it. How could this be optimized?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Stack
{
    class StackElement<T>
    {
        public T Data { get; set; }
        public StackElement<T> Below { get; set; }
        public StackElement(T data)
        {
            Data = data;
        }
    }

    public class Stack<T>
    {
        private StackElement<T> top;

        public void Push(T item)              
        {
            StackElement<T> temp;
            if (top == null)
            {
                top = new StackElement<T>(item);
            }
            else
            {
                temp = top;
                top = new StackElement<T>(item);
                top.Below = temp;                
            }
        }

        public T Pop()
        {
            if (top == null)
            {
                throw new Exception("Sorry, nothing on the stack");
            }
            else
            {
                T temp = top.Data;                
                top = top.Below;
                return temp;
            }        
        }

        public void Clear()
        {
            while (top != null)
                Pop();
        }

    }


    class TestProgram
    {
        static void Main(string[] args)
        {
            Test1();
            Test2();
            Test3();
        }

        private static void Test1()
        { 
            Stack<string> myStack = new Stack<string>();
            myStack.Push("joe");
            myStack.Push("mike");
            myStack.Push("adam");

            if (myStack.Pop() != "adam") { throw new Exception("fail"); }
            if (myStack.Pop() != "mike") { throw new Exception("fail"); }
            if (myStack.Pop() != "joe") { throw new Exception("fail"); }

        }

        private static void Test3()
        {

            Stack<string> myStack = new Stack<string>();
            myStack.Push("joe");
            myStack.Push("mike");
            myStack.Push("adam");
            myStack.Clear();
            try
            {
                myStack.Pop();

            }
            catch (Exception ex)
            {
                return;
            }

            throw new Exception("fail");
        }

        private static void Test2()
        {
            Stack<string> myStack = new Stack<string>();
            myStack.Push("joe");
            myStack.Push("mike");
            myStack.Push("adam");

            if (myStack.Pop() != "adam") { throw new Exception("fail"); }
            myStack.Push("alien");
            myStack.Push("nation");
            if (myStack.Pop() != "nation") { throw new Exception("fail"); }
            if (myStack.Pop() != "alien") { throw new Exception("fail"); }

        }

    }
}

Answer 1

I think the Clear() method could be sped up significantly by changing it to top = null;. The entire stack will be garbage collected, and no loop required in the mean time.

Answer 2

You could simply use an array. The .NET array methods are really fast.

public class Stack<T>
{
    private const int _defaultSize = 4;
    private const int _growthMultiplier = 2;

    private T[] _elements;
    private int _index;
    private int _limit;


    public Stack()
    {
        _elements = new T[_defaultSize];
        _index = -1;
        _limit = _elements.Length - 1;
    }


    public void Push(T item)
    {
        if (_index == _limit)
        {
            var temp = _elements;
            _elements = new T[_elements.Length * _growthMultiplier];
            _limit = _elements.Length - 1;
            Array.Copy(temp, _elements, temp.Length);
        }
        _elements[++_index] = item;
    }

    public T Pop()
    {
        if (_index < 0)
            throw new InvalidOperationException();

        var item = _elements[_index];
        _elements[_index--] = default(T);
        return item;
    }

    public void Clear()
    {
        _index = -1;
        Array.Clear(_elements, 0, _elements.Length);
    }
}

Answer 3

Might be preferable to use dynamic array as the data structure instead of a linked list. The array will have better locality of reference because the elements are next to each other. A stack doesn't need ability to delete elements in the middle, splicing etc. so an array suffices.