Krdytkyu

I noticed a few things that can be improved:

• Not all enumerators support Reset. Generator methods don't, for example, so calling ZipNew on the result of a ZipNew call will fail with a NotSupportedException. Obtaining a new enumerator should work, at the cost of having to replace the convenient using statements with try/finally constructions. Edit: As Eric pointed out, Reset should not be used at all. It's been abandoned.


• There's no need to call Reset (or rather, to get a new enumerator) when a collection is empty. I'd probably add a special case for that.


• Passing null causes either an unspecific NullReferenceException or an ArgumentNullException with parameter name source to be thrown. Throwing ArgumentNullExceptions with accurate parameter names would be more helpful. Edit: As JAD pointed out, this is trickier than it looks. You'll have to split the method into an eager non-yielding method and a lazy yielding method. A local function should be useful here.


• 
  i1 and i2 can be declared inside the while loop.

Addendum:

As Henrik's answer shows, a helper class can be useful for properly repeating enumerators without having to give up on using. I would take a slightly different approach by creating a repeatable enumerator class:

class RepeatableEnumerator<T> : IDisposable

{

    private IEnumerable<T> _enumerable;

    private IEnumerator<T> _enumerator;





    public bool IsRepeating { get; private set; }

    public T Current => _enumerator.Current;





    public RepeatableEnumerator(IEnumerable<T> enumerable)

    {

        _enumerable = enumerable;

        _enumerator = enumerable.GetEnumerator();

    }



    public void Dispose()

    {

        _enumerator.Dispose();

        _enumerator = null;

    }



    public bool MoveNext() => _enumerator.MoveNext();



    public bool Repeat()

    {

        IsRepeating = true;

        _enumerator.Dispose();

        _enumerator = _enumerable.GetEnumerator();

        return _enumerator.MoveNext();

    }

}

Which can then be used for both enumerables (and possibly in other extension methods as well):

public static IEnumerable<TResult> ZipLongest<TFirst, TSecond, TResult>(

    this IEnumerable<TFirst> first,

    IEnumerable<TSecond> second,

    Func<TFirst, TSecond, TResult> resultSelector)

{

    // Eager parameter validation:

    if (first == null) throw new ArgumentNullException(nameof(first));

    if (second == null) throw new ArgumentNullException(nameof(second));

    if (resultSelector == null) throw new ArgumentNullException(nameof(resultSelector));



    // Local function for lazy zipping:

    IEnumerable<TResult> ZipLongestImpl()

    {

        using (var enum1 = new RepeatableEnumerator<TFirst>(first))

        using (var enum2 = new RepeatableEnumerator<TSecond>(second))

        {

            // Up-front check for empty collections:

            if (!enum1.MoveNext() || !enum2.MoveNext())

                yield break;



            while (true)

            {

                yield return resultSelector(enum1.Current, enum2.Current);



                var is1Empty = !enum1.MoveNext();

                var is2Empty = !enum2.MoveNext();

                if (is1Empty)

                {

                    if (enum2.IsRepeating || is2Empty || !enum1.Repeat())

                        yield break;

                }

                else if (is2Empty)

                {

                    if (enum1.IsRepeating || !enum2.Repeat())

                        yield break;

                }

            }

        }

    }

    return ZipLongestImpl();

}

At this point it would be a good idea to add some documentation...

    public static class Impl

    {

        public static IEnumerable<TResult> ZipNew<TFirst, TSecond, TResult>(

Names? The class would be more descriptive as something like LinqExtensions; the method something like ZipLooped.

            using (IEnumerator<TFirst> iterator1 = first.GetEnumerator()) 

            using (IEnumerator<TSecond> iterator2 = second.GetEnumerator())

            {

                var i1 = true;

                var i2 = true;

                var i1Shorter = false;

                var i2Shorter = false;

                var firstRun = true;

The iterators have useful names, but what does i1 mean? And why five variables to track the state of two iterators? IMO it would be simpler as

                var firstEnded = false;

                var secondEnded = false;



                while (true) 

                {

                    if (!iterator1.MoveNext())

                    {

                        if (secondEnded) yield break;

                        firstEnded = true;

                        iterator1.Reset();

                        if (!iterator1.MoveNext()) yield break;

                    }

                    if (!iterator2.MoveNext())

                    {

                        if (firstEnded) yield break;

                        secondEnded = true;

                        iterator2.Reset();

                        if (!iterator2.MoveNext()) yield break;

                    }



                    yield return resultSelector(iterator1.Current, iterator2.Current); 

                }

and the almost repeated code might be worth pulling out as an inner method:

                var firstEnded = false;

                var secondEnded = false;



                bool advance<T>(IEnumerator<T> it, ref bool thisEnded, bool otherEnded)

                {

                    if (it.MoveNext()) return true;

                    // `it` has done a full cycle; if the other one has too, we've finished

                    if (otherEnded) return false;

                    thisEnded = true;

                    // Start again, although if `it` is empty we need to abort

                    it.Reset();

                    return it.MoveNext();

                }



                while (true)

                {

                    if (!advance(iterator1, ref firstEnded, secondEnded)) yield break;

                    if (!advance(iterator2, ref secondEnded, firstEnded)) yield break;

                    yield return resultSelector(iterator1.Current, iterator2.Current); 

                }

I notice that you've decided to yield break if either of the enumerables is empty. Would an exception be a better choice?

If you should respect that not all Enumerators implement Reset() then it is not possible to use using statements for the two IEnumerators. But you could introduce an IEnumerator<TResult> for the zipped result and use it like this:

public static IEnumerable<TResult> ZipNew<TFirst, TSecond, TResult>(

this IEnumerable<TFirst> first,

IEnumerable<TSecond> second,

Func<TFirst, TSecond, TResult> resultSelector)

{

  if (first == null) throw new ArgumentNullException(nameof(first));

  if (second == null) throw new ArgumentNullException(nameof(second));

  if (resultSelector == null) throw new ArgumentNullException(nameof(resultSelector));



  return InnerZipNew(first, second, resultSelector);

}



private static IEnumerable<TResult> InnerZipNew<TFirst, TSecond, TResult>(

this IEnumerable<TFirst> first,

IEnumerable<TSecond> second,

Func<TFirst, TSecond, TResult> resultSelector)

{

  using (ZipEnumerator<TFirst, TSecond, TResult> zipEnumerator = new ZipEnumerator<TFirst, TSecond, TResult>(first, second, resultSelector))

  {

    while (zipEnumerator.MoveNext())

    {

      yield return zipEnumerator.Current;

    }

  }

}

As JAD writes in his comment it is necessary to catch possible invalid input as the first thing and then call a private shadow method to do the actual iteration in order to make the exceptions be thrown when the extension is called rather than when the enumeration is performed.

In this way you're back on the using track.

The ZipEnumerator it self could be something like:

  public class ZipEnumerator<T, S, TResult> : IEnumerator<TResult>

  {

    IEnumerable<T> m_dataT;

    IEnumerable<S> m_dataS;

    IEnumerator<T> m_enumerT;

    IEnumerator<S> m_enumerS;

    List<IDisposable> m_disposables = new List<IDisposable>();

    Func<T, S, TResult> m_selector;

    bool m_secondReloaded = false;

    bool m_first = true;



    public ZipEnumerator(IEnumerable<T> dataT, IEnumerable<S> dataS, Func<T, S, TResult> selector)

    {

      m_dataT = dataT ?? throw new ArgumentNullException(nameof(dataT));

      m_dataS = dataS ?? throw new ArgumentNullException(nameof(dataS));

      m_selector = selector ?? throw new ArgumentNullException(nameof(selector));



    }



    public TResult Current => m_selector(m_enumerT.Current, m_enumerS.Current);



    object IEnumerator.Current => Current;



    public void Dispose()

    {

      foreach (IDisposable disposable in m_disposables)

      {

        disposable.Dispose();

      }

      m_disposables.Clear();

    }



    private IEnumerator<T> GetTEnumerator()

    {

      var enumerator = m_dataT.GetEnumerator();

      m_disposables.Add(enumerator);

      return enumerator;

    }



    private IEnumerator<S> GetSEnumerator()

    {

      var enumerator = m_dataS.GetEnumerator();

      m_disposables.Add(enumerator);

      return enumerator;

    }



    public bool MoveNext()

    {

      m_enumerT = m_enumerT ?? GetTEnumerator();

      m_enumerS = m_enumerS ?? GetSEnumerator();



      if (m_first)

      {

        if (m_enumerT.MoveNext())

        {

          if (!m_enumerS.MoveNext())

          {

            m_enumerS = GetSEnumerator();

            m_secondReloaded = true;

            if (!m_enumerS.MoveNext())

              return false;

          }

          return true;

        }

        else

        {

          m_first = false;

        }

      }



      if (!m_first && !m_secondReloaded)

      {

        if (m_enumerS.MoveNext())

        {

          if (!m_enumerT.MoveNext())

          {

            m_enumerT = GetTEnumerator();

            if (!m_enumerT.MoveNext())

              return false;

          }



          return true;

        }

      }



      return false;

    }



    public void Reset()

    {

      m_secondReloaded = false;

      m_first = true;

      m_enumerT = null;

      m_enumerS = null;

      Dispose();

    }

  }

It's a little more code than other suggestions, but it encapsulates the problems with the disposal of intermediate enumerators without the necessity of a try-catch-statement. You could discuss if the disposal should be immediately when the enumerator is done or as I do collect them for disposal when the ZipEnumerator itself is disposed off?

The MoveNext() method went a little more complicated than I like, so feel free to edit or suggest improvements.

Edit

A refactored version of ZipEnumerator:

  public class ZipEnumerator<T, S, TResult> : IEnumerator<TResult>

  {

    IEnumerable<T> m_dataT;

    IEnumerable<S> m_dataS;

    IEnumerator<T> m_enumeratorT;

    IEnumerator<S> m_enumeratorS;

    List<IDisposable> m_disposables = new List<IDisposable>();

    Func<T, S, TResult> m_selector;

    bool m_secondReloaded = false;

    bool m_isInitilized = false;



    public ZipEnumerator(IEnumerable<T> dataT, IEnumerable<S> dataS, Func<T, S, TResult> selector)

    {

      m_dataT = dataT ?? throw new ArgumentNullException(nameof(dataT));

      m_dataS = dataS ?? throw new ArgumentNullException(nameof(dataS));

      m_selector = selector ?? throw new ArgumentNullException(nameof(selector));

    }



    public TResult Current => m_selector(m_enumeratorT.Current, m_enumeratorS.Current);

    object IEnumerator.Current => Current;



    public void Dispose()

    {

      DoDispose();

    }



    private void RegisterDisposable(IDisposable disposable)

    {

      m_disposables.Add(disposable);

      if (m_disposables.Count > 10)

      {

        DoDispose();

      }

    }



    private void DoDispose()

    {

      foreach (IDisposable disposable in m_disposables)

      {

        disposable.Dispose();

      }

      m_disposables.Clear();

    }



    private IEnumerator<T> GetTEnumerator()

    {

      var enumerator = m_dataT.GetEnumerator();

      RegisterDisposable(enumerator);

      return enumerator;

    }



    private IEnumerator<S> GetSEnumerator()

    {

      var enumerator = m_dataS.GetEnumerator();

      RegisterDisposable(enumerator);

      return enumerator;

    }



    private Func<bool> CurrentMover = null;



    private bool FirstMover()

    {

      if (m_enumeratorT.MoveNext())

      {

        if (!m_enumeratorS.MoveNext())

        {

          m_enumeratorS = GetSEnumerator();

          m_secondReloaded = true;

          if (!m_enumeratorS.MoveNext())

            return false;

        }

        return true;

      }

      else if (!m_secondReloaded)

      {

        CurrentMover = SecondMover;

        return CurrentMover();

      }



      return false;

    }



    private bool SecondMover()

    {

      if (m_enumeratorS.MoveNext())

      {

        if (!m_enumeratorT.MoveNext())

        {

          m_enumeratorT = GetTEnumerator();

          if (!m_enumeratorT.MoveNext())

            return false;

        }



        return true;

      }



      return false;

    }



    private void Initialize()

    {

      m_enumeratorT = GetTEnumerator();

      m_enumeratorS = GetSEnumerator();

      CurrentMover = FirstMover;

      m_isInitilized = true;

    }



    public bool MoveNext()

    {

      if (!m_isInitilized)

      {

        Initialize();

      }

      return CurrentMover();

    }



    public void Reset()

    {

      m_isInitilized = false;

      m_secondReloaded = false;

      CurrentMover = null;

      m_enumeratorT = null;

      m_enumeratorS = null;

      DoDispose();

    }

  }