Simple linked list with two iterators

Question

As part of the practice, I created minimalist linked list named SimpleList which can be iterated in two different ways: one element after another or by every second element.

Because I'm beginner I would really appreciate your opinion about my solution. Is it correct? What should I change?

public class SimpleList<T> : IEnumerable<T>
{
    protected class Node
    {
        public T Data { get; set; }
        public Node NextNode { get; set; }

        public Node(T data, Node nextNode)
        {
            Data = data;
            NextNode = nextNode;
        }
    }

    protected Node Head { get; set; } = null;

    public void Add(T data)
    {
        Head = new Node(data, Head);
    }

    #region IEnumerable<T> implementation
    public IEnumerator<T> GetEnumerator()
    {
        var current = Head;
        while (current != null)
        {
            yield return current.Data;
            current = current.NextNode;
        }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
    #endregion

    #region second iterator implementation
    private class EverySecondElementIterator : IEnumerable<T>
    {
        private readonly Node head;

        public EverySecondElementIterator(Node head)
        {
            this.head = head;
        }

        public IEnumerator<T> GetEnumerator()
        {
            var current = head;
            while (current != null)
            {
                yield return current.Data;
                current = current.NextNode?.NextNode;
            }
        }

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

    public IEnumerable<T> EverySecondElement => new EverySecondElementIterator(Head);
    #endregion
}

Answer 1

Use extension methods

I find you did well by implementing the EverySecondElementIterator as a separate class. However, I also think it would be better to turn it into an extension of IEnumerable<T> and use MoveNext instead of the internal Node.

Otherwise if someone suddenly comes up with the idea to skip every third element, you would need to modify the original SimpleList by creating another private class class and another property. We usually try to avoid modifying code that is tested and is working well. This would also violate the open/closed principle.

It's much safer and extendable to create a new and independent extension that you also can resue with any IEnumerable<T> and skip every-second-element of any collection.

Let the SimpleList be closed for modifications by not implementing anything as specific as that enumerator. It is already opend for extension by implementing the IEnumerable<T> interface. Make use of it.

Answer 2

Terminology

The Add operation on a List appends the item to that list. Your method should be renamed to Prepend.

 public void Add(T data)
 {
      Head = new Node(data, Head);
 }

 public void Prepend(T data)
 {
      Head = new Node(data, Head);
 }

Design

You could keep track of the Tail and have an Add operation.

protected Node Head { get; set; } = null;

 protected Node Head { get; set; } = null;
 protected Node Tail { get { 
     var tail = Head;
     while (tail != null && tail.NextNode != null)
         tail = tail.NextNode;
     return tail;
 }}

 public void Add(T data)
 {
     if (Head == null)
         Head = new Node(data, null);
     else
         Tail.NextNode = new Node(data, null);
 }