MinHeap implementation

Please comment on this.

class MinHeap where T: IComparable
    {
        List elements;

        public MinHeap()
        {
            elements = new List();
        }

        public void Add(T item)
        {
            elements.Add(item);
            Heapify();
        }

        public void Delete(T item)
        {
            int i = elements.IndexOf(item);
            int last = elements.Count - 1;

            elements[i] = elements[last];
            elements.RemoveAt(last);
            Heapify();
        }

        public T PopMin()
        {
            if (elements.Count > 0)
            {
                T item = elements[0];
                Delete(item);
                return item;
            }
            //relook at this - should we just throw exception?
            return default(T);
        }

        public void Heapify()
        {
            for (int i = elements.Count - 1; i > 0; i--)
            {                
                int parentPosition = (i+1) / 2 - 1;
                parentPosition = parentPosition >= 0 ? parentPosition : 0;

                if (elements[parentPosition].CompareTo(elements[i])>1)
                {
                    T tmp = elements[parentPosition];
                    elements[parentPosition] = elements[i];
                    elements[i] = tmp;
                }
            }
        }
    }

-
You should always try to program against an interface instead of against an implementation.

List elements;

should be

IList elements;

-
Restrict the scope of fields and methods to the smallest possible.

So public void Heapify() should be private instead.

-
The swapping of elements inside the Heapify() method can be extracted to a private method.

private void SwapElements(int firstIndex, int secondIndex)
{
    T tmp = elements[firstIndex];
    elements[firstIndex] = elements[secondIndex];
    elements[secondIndex] = tmp;
}

Former Heapify() method now

private void Heapify()
{
    for (int i = elements.Count - 1; i > 0; i--)
    {                
        int parentPosition = (i+1) / 2 - 1;
        parentPosition = parentPosition >= 0 ? parentPosition : 0;

        if (elements[parentPosition].CompareTo(elements[i]) > 1)
        {
            SwapElements(parentPosition, i)
        }
    }
}

-
Potential bug

You are checking if the CompareTo() method returns a value > 1 but you miss ==1 as the documentation states

• value This instance precedes obj in the sort order.

• value == 0 => This instance occurs in the same position in the sort order as obj.

• value > 0 => This instance follows obj in the sort order.

-
//relook at this - should we just throw exception?

If you want to simulate a Stack then you should throw an InvalidOperationException but only if you also provide a property Count so that a user of your class can evaluate if he can call PopMin safely.

StackExchange Code Review Q#68530, answer score: 7