C#: SkipLast implementation

Question

I needed a method to give me all but the last item in a sequence. This is my current implementation:

    public static IEnumerable<T> SkipLast<T>(this IEnumerable<T> source)
    {
        using (IEnumerator<T> iterator = source.GetEnumerator())
        {
            if(iterator.MoveNext())
                while(true)
                {
                    var current = iterator.Current;
                    if(!iterator.MoveNext())
                        yield break;
                    yield return current;
                }
        }
    }

What I need it for is to do something with all the items except the last one. In my case I have a sequence of objects with various properties. I then order them by date, and then I need to do an adjustment to all of them except the most recent item (which would be the last one after ordering).

Thing is, I am not too into these enumerators and stuff yet and don't really have anyone here to ask either :p What I am wondering is if this is a good implementation, or if I have done a small or big blunder somewhere. Or if maybe this take on the problem is a weird one, etc.

I guess a more general implementation could have been an AllExceptMaxBy method. Since that is kind of what it is. The MoreLinq has a MaxBy and MinBy method and my method kind of need to do the same, but return every item except the maximum or minimum one.

Answer 1

This is tricky, as "last element" isn't a Markov stopping point: you can't tell that you've got to the last element until you try to get the next one. It's doable, but only if you don't mind permanently being "one element behind". That's basically what your current implementation does, and it looks okay, although I'd probably write it slightly differently.

An alternative approach would be to use foreach, always yielding the previously returned value unless you were at the first iteration:

public static IEnumerable<T> SkipLast<T>(this IEnumerable<T> source)
{
    T previous = default(T);
    bool first = true;
    foreach (T element in source)
    {
        if (!first)
        {
            yield return previous;
        }
        previous = element;
        first = false;
    }
}

Another option, closer to your code:

public static IEnumerable<T> SkipLast<T>(this IEnumerable<T> source)
{
    using (IEnumerator<T> iterator = source.GetEnumerator())
    {
        if(!iterator.MoveNext())
        {
            yield break;
        }
        T previous = iterator.Current;
        while (iterator.MoveNext())
        {
            yield return previous;
            previous = iterator.Current;
        }
    }
}

That avoids nesting quite as deeply (by doing an early exit if the sequence is empty) and it uses a "real" while condition instead of while(true)

Answer 2

Your implementation looks perfectly fine to me - it's probably the way I would do it.

The only simplification I might suggest in relation to your situation is to order the list the other way round (i.e. ascending rather than descending). Although this may not be suitable in your code, it would allow you to simply use collection.Skip(1) to take all items except the most recent one.

If this isn't possible for reasons you haven't shown in your post, then your current implementation is no problem at all.

Answer 3

If you're using .NET 3.5, I guess you could use:

public static IEnumerable<T> SkipLast<T>(this IEnumerable<T> source)
{
  return source.TakeWhile((item, index) => index < source.Count() - 1))
}

Answer 4

public static IEnumerable<T> SkipLast<T>(this IEnumerable<T> source)
{
    if (!source.Any())
    {
        yield break;
    }
    Queue<T> items = new Queue<T>();
    items.Enqueue(source.First());
    foreach(T item in source.Skip(1))
    {
        yield return items.Dequeue();
        items.Enqueue(item);
    }
}

Answer 5

(Old answer scrapped; this code has been tested and works.) It prints
first
second
FIRST
SECOND
THIRD

public static class ExtNum{
  public static IEnumerable skipLast(this IEnumerable source){
    if ( ! source.Any())
      yield break;
    for (int i = 0 ; i <=source.Count()-2 ; i++ )
      yield return source.ElementAt(i);
    yield break;
  }
}
class Program
{
  static void Main( string[] args )
  {
    Queue qq = new Queue();
    qq.Enqueue("first");qq.Enqueue("second");qq.Enqueue("third");
    List lq = new List();
    lq.Add("FIRST"); lq.Add("SECOND"); lq.Add("THIRD"); lq.Add("FOURTH");
    foreach(string s1 in qq.skipLast())
      Console.WriteLine(s1);
    foreach ( string s2 in lq.skipLast())
      Console.WriteLine(s2);
  }
}