SkipLast of an IEnumerable - Linq ExtensionMy own implementation of Linq SelectMany extension methodA genric extension method to filter Linq-EF queriesLazy man's IEnumerable extension verification methodExtension method to do Linq LookupsConverting from binary to unaryIEnumerable extension method that ingests SqlCommand and returns query resultsIEnumerable Extensions Linq AllOrDefault() Each()IQueryable left outer join linq extensionExtension method replacing elements from an IEnumerable<T> collection follow upLinq IEnumerable extensions for void functions

Why do airports remove/realign runways?

What exactly is a "murder hobo"?

What was the significance of Spider-Man: Far From Home being an MCU Phase 3 film instead of a Phase 4 film?

How was the website able to tell my credit card was wrong before it processed it?

Is this really the Saturn V computer only, or are there other systems here as well?

This LM317 diagram doesn't make any sense to me

Is there a method for differentiating informative comments from commented out code?

My professor has told me he will be the corresponding author. Will it hurt my future career?

Who goes first? Person disembarking bus or the bicycle?

How can I use my cell phone's light as a reading light?

How to evaluate the performance of open source solver?

Four ships at the ocean with the same distance

How do resistors generate different heat if we make the current fixed and changed the voltage and resistance? Notice the flow of charge is constant

Appropriate conduit for several data cables underground over 300' run

What does the multimeter dial do internally?

What is the average number of draws it takes before you can not draw any more cards from the Deck of Many Things?

What factors could lead to bishops establishing monastic armies?

Why did Old English lose both thorn and eth?

Chilling water in copper vessel

Is it possible to complete a PhD in CS in 3 years?

How to understand flavors and when to use combination of them?

Why won't the U.S. sign a peace treaty with North Korea?

With a data transfer of 50 GB estimated 5 hours, are USB-C claimed speeds inaccurate or to blame?

As a supervisor, what feedback would you expect from a PhD who quits?



SkipLast of an IEnumerable - Linq Extension


My own implementation of Linq SelectMany extension methodA genric extension method to filter Linq-EF queriesLazy man's IEnumerable extension verification methodExtension method to do Linq LookupsConverting from binary to unaryIEnumerable extension method that ingests SqlCommand and returns query resultsIEnumerable Extensions Linq AllOrDefault() Each()IQueryable left outer join linq extensionExtension method replacing elements from an IEnumerable<T> collection follow upLinq IEnumerable extensions for void functions






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








2












$begingroup$


As my answer to this question, I came up with this solution:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)



It returns all items in the set but the count last, but without knowing anything about the size of the data set. I think it's funny and that it's working, but a comment claims that is doesn't. Am I overlooking something?




A version with a circular queue could be:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)
count < 0) yield break;
if (count == 0)

foreach (T item in data)
yield return item;

else

T[] queue = data.Take(count).ToArray();
int index = 0;

foreach (T item in data.Skip(count))

index %= count;
yield return queue[index];
queue[index] = item;
index++;





Performance wise they seems to be even.




Compared to other solutions like the most obvious:



data.Reverse().Skip(count).Reverse()


it seems to be at least as fast and for very large set about twice as fast.



Test case:



 int count = 20;
var data = Enumerable.Range(1, count);

for (int i = 0; i < count + 5; i++)

Console.WriteLine($"Skip: i => (string.Join(", ", data.SkipLast1(i)))");



Any comments are useful.










share|improve this question











$endgroup$







  • 1




    $begingroup$
    Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
    $endgroup$
    – t3chb0t
    13 hours ago










  • $begingroup$
    @t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
    $endgroup$
    – Henrik Hansen
    13 hours ago







  • 1




    $begingroup$
    I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
    $endgroup$
    – t3chb0t
    13 hours ago


















2












$begingroup$


As my answer to this question, I came up with this solution:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)



It returns all items in the set but the count last, but without knowing anything about the size of the data set. I think it's funny and that it's working, but a comment claims that is doesn't. Am I overlooking something?




A version with a circular queue could be:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)
count < 0) yield break;
if (count == 0)

foreach (T item in data)
yield return item;

else

T[] queue = data.Take(count).ToArray();
int index = 0;

foreach (T item in data.Skip(count))

index %= count;
yield return queue[index];
queue[index] = item;
index++;





Performance wise they seems to be even.




Compared to other solutions like the most obvious:



data.Reverse().Skip(count).Reverse()


it seems to be at least as fast and for very large set about twice as fast.



Test case:



 int count = 20;
var data = Enumerable.Range(1, count);

for (int i = 0; i < count + 5; i++)

Console.WriteLine($"Skip: i => (string.Join(", ", data.SkipLast1(i)))");



Any comments are useful.










share|improve this question











$endgroup$







  • 1




    $begingroup$
    Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
    $endgroup$
    – t3chb0t
    13 hours ago










  • $begingroup$
    @t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
    $endgroup$
    – Henrik Hansen
    13 hours ago







  • 1




    $begingroup$
    I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
    $endgroup$
    – t3chb0t
    13 hours ago














2












2








2





$begingroup$


As my answer to this question, I came up with this solution:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)



It returns all items in the set but the count last, but without knowing anything about the size of the data set. I think it's funny and that it's working, but a comment claims that is doesn't. Am I overlooking something?




A version with a circular queue could be:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)
count < 0) yield break;
if (count == 0)

foreach (T item in data)
yield return item;

else

T[] queue = data.Take(count).ToArray();
int index = 0;

foreach (T item in data.Skip(count))

index %= count;
yield return queue[index];
queue[index] = item;
index++;





Performance wise they seems to be even.




Compared to other solutions like the most obvious:



data.Reverse().Skip(count).Reverse()


it seems to be at least as fast and for very large set about twice as fast.



Test case:



 int count = 20;
var data = Enumerable.Range(1, count);

for (int i = 0; i < count + 5; i++)

Console.WriteLine($"Skip: i => (string.Join(", ", data.SkipLast1(i)))");



Any comments are useful.










share|improve this question











$endgroup$




As my answer to this question, I came up with this solution:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)



It returns all items in the set but the count last, but without knowing anything about the size of the data set. I think it's funny and that it's working, but a comment claims that is doesn't. Am I overlooking something?




A version with a circular queue could be:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)
count < 0) yield break;
if (count == 0)

foreach (T item in data)
yield return item;

else

T[] queue = data.Take(count).ToArray();
int index = 0;

foreach (T item in data.Skip(count))

index %= count;
yield return queue[index];
queue[index] = item;
index++;





Performance wise they seems to be even.




Compared to other solutions like the most obvious:



data.Reverse().Skip(count).Reverse()


it seems to be at least as fast and for very large set about twice as fast.



Test case:



 int count = 20;
var data = Enumerable.Range(1, count);

for (int i = 0; i < count + 5; i++)

Console.WriteLine($"Skip: i => (string.Join(", ", data.SkipLast1(i)))");



Any comments are useful.







c# comparative-review linq extension-methods






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited 13 hours ago









t3chb0t

36.4k7 gold badges58 silver badges135 bronze badges




36.4k7 gold badges58 silver badges135 bronze badges










asked 14 hours ago









Henrik HansenHenrik Hansen

10.3k1 gold badge13 silver badges35 bronze badges




10.3k1 gold badge13 silver badges35 bronze badges







  • 1




    $begingroup$
    Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
    $endgroup$
    – t3chb0t
    13 hours ago










  • $begingroup$
    @t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
    $endgroup$
    – Henrik Hansen
    13 hours ago







  • 1




    $begingroup$
    I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
    $endgroup$
    – t3chb0t
    13 hours ago













  • 1




    $begingroup$
    Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
    $endgroup$
    – t3chb0t
    13 hours ago










  • $begingroup$
    @t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
    $endgroup$
    – Henrik Hansen
    13 hours ago







  • 1




    $begingroup$
    I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
    $endgroup$
    – t3chb0t
    13 hours ago








1




1




$begingroup$
Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
$endgroup$
– t3chb0t
13 hours ago




$begingroup$
Ignore my delete answer. I missed one small detail. I like that version with the queue better because it doesn't require this complex index manipulations and has only one loop.
$endgroup$
– t3chb0t
13 hours ago












$begingroup$
@t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
$endgroup$
– Henrik Hansen
13 hours ago





$begingroup$
@t3chb0t: but you have a point in that if countis large, the queue gets large, resulting in a large memory consumption an the overhead of copying the data.
$endgroup$
– Henrik Hansen
13 hours ago





1




1




$begingroup$
I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
$endgroup$
– t3chb0t
13 hours ago





$begingroup$
I think it'll rarely get larger than 1 so your caching techinique with the Queue is very cool :-) if count it would get so large that it would cause memory issues then there is a much bigger issue with the application logic. Or can you think of any example when it would make sense to use SkipLast(1_000_000)?
$endgroup$
– t3chb0t
13 hours ago











3 Answers
3






active

oldest

votes


















2












$begingroup$


if (data == null || count < 0) yield break;



This behaviour is somewhat consistent with Take, but not with Skip: Skip treats a negative values as a zero. As, indeed, does the SkipLast which doesn't appear in .NET Framework.



It should throw on a null argument with an ArgumentNullException.




My only other real issue with the methods is that neither will work with IEnumerables that can't be enumerated multiple times, and will incur overheads in any that can but have to generate the data lazily.



I would go for the slightly more painful:





if (source == null)
throw new ArgumentNullException("Source Enumeration may not be null", nameof(source));

if (count <= 0)

foreach (T item in source)
yield return item;

else

bool yielding = false;
T[] buffer = new T[count];
int index = 0;

foreach (T item in source)

if (index == count)

index = 0;
yielding = true;


if (yielding)
yield return buffer[index];

buffer[index] = item;
index++;




If I cared about performance, I might consider the following, which reduces the amount of decision making inside the loop (which might make it faster: I'd better benchmark it).



// just the bit inside the else
T[] buffer = new T[count];

using (var e = source.GetEnumerator())

// initial filling of buffer
for (int i = 0; i < buffer.Length; i++)

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

buffer[i] = e.Current;


int index = 0;
while (e.MoveNext())

yield return buffer[index];
buffer[index] = e.Current;
index = (index + 1) % count;






Performance wise they seems to be even.




That's encouraging, since Queue<T> is also implemented as a circular buffer. You'd hope that the array based version would be a bit lighter, but may consume more memory is Count > data.Count().



Having benchmarked your two proposals, my two proposals, and the .NET Core SkipLast (didn't include the Reverse based method), it seems the fastest methods are that built into .NET Core (hurray) and my last one, but the difference between test instance (with different data lengths and skip counts) is great. Unfortunately, I messed up and didn't run a third of the .NET Core tests, so the saga of incompetence on my part continues. The code and data can be found in a gist. The only real conclusion I would want to draw from this data (aside from 'use the BCL method if you can') is that your first method is consistently the slowest when the input array isn't empty in these tests on my machine with it's current workload. The difference is jolly significant, with your first method requiring twice as much time as others in some cases. Why the methods have different performance is less than clear.






share|improve this answer











$endgroup$












  • $begingroup$
    Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
    $endgroup$
    – Henrik Hansen
    1 hour ago


















2












$begingroup$

There are 2 cases for which both your and VisualMelon's implementation can be improved:



  • If count is less than or equal to 0, then you can return source directly. This speeds up iteration by removing an unnecessary intermediate step. For this to work, the yielding part of the method has to be moved into another method, but that's easy with local functions.

  • If source is ICollection<T> collection, then you can return source.Take(collection.Count - count), which is faster and uses less memory because it doesn't need to buffer anything. The higher count is, the more of a difference this makes.

The first of these is probably not a very useful edge-case, so it might not be worth the extra code, but I would definitely include the second optimization.






share|improve this answer









$endgroup$












  • $begingroup$
    +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
    $endgroup$
    – VisualMelon
    5 hours ago










  • $begingroup$
    I have actually made a version after my post that is implemented as your first suggestion.
    $endgroup$
    – Henrik Hansen
    1 hour ago


















0












$begingroup$

Takning all considerations by VisualMelon and Pieter Witvoet into account a solution could now be:



static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)

if (data == null) throw new ArgumentNullException(nameof(data));
if (count <= 0) return data;

if (data is ICollection<T> collection)
return collection.Take(collection.Count - count);

IEnumerable<T> Skipper()

using (var enumer = data.GetEnumerator())

T[] queue = new T[count];
int index = 0;

while (index < count && enumer.MoveNext())
queue[index++] = enumer.Current;

index = -1;
while (enumer.MoveNext())

index = (index + 1) % count;
yield return queue[index];
queue[index] = enumer.Current;




return Skipper();






share|improve this answer











$endgroup$















    Your Answer






    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    autoActivateHeartbeat: false,
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader:
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    ,
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );













    draft saved

    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223627%2fskiplast-of-an-ienumerablet-linq-extension%23new-answer', 'question_page');

    );

    Post as a guest















    Required, but never shown

























    3 Answers
    3






    active

    oldest

    votes








    3 Answers
    3






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    2












    $begingroup$


    if (data == null || count < 0) yield break;



    This behaviour is somewhat consistent with Take, but not with Skip: Skip treats a negative values as a zero. As, indeed, does the SkipLast which doesn't appear in .NET Framework.



    It should throw on a null argument with an ArgumentNullException.




    My only other real issue with the methods is that neither will work with IEnumerables that can't be enumerated multiple times, and will incur overheads in any that can but have to generate the data lazily.



    I would go for the slightly more painful:





    if (source == null)
    throw new ArgumentNullException("Source Enumeration may not be null", nameof(source));

    if (count <= 0)

    foreach (T item in source)
    yield return item;

    else

    bool yielding = false;
    T[] buffer = new T[count];
    int index = 0;

    foreach (T item in source)

    if (index == count)

    index = 0;
    yielding = true;


    if (yielding)
    yield return buffer[index];

    buffer[index] = item;
    index++;




    If I cared about performance, I might consider the following, which reduces the amount of decision making inside the loop (which might make it faster: I'd better benchmark it).



    // just the bit inside the else
    T[] buffer = new T[count];

    using (var e = source.GetEnumerator())

    // initial filling of buffer
    for (int i = 0; i < buffer.Length; i++)

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

    buffer[i] = e.Current;


    int index = 0;
    while (e.MoveNext())

    yield return buffer[index];
    buffer[index] = e.Current;
    index = (index + 1) % count;






    Performance wise they seems to be even.




    That's encouraging, since Queue<T> is also implemented as a circular buffer. You'd hope that the array based version would be a bit lighter, but may consume more memory is Count > data.Count().



    Having benchmarked your two proposals, my two proposals, and the .NET Core SkipLast (didn't include the Reverse based method), it seems the fastest methods are that built into .NET Core (hurray) and my last one, but the difference between test instance (with different data lengths and skip counts) is great. Unfortunately, I messed up and didn't run a third of the .NET Core tests, so the saga of incompetence on my part continues. The code and data can be found in a gist. The only real conclusion I would want to draw from this data (aside from 'use the BCL method if you can') is that your first method is consistently the slowest when the input array isn't empty in these tests on my machine with it's current workload. The difference is jolly significant, with your first method requiring twice as much time as others in some cases. Why the methods have different performance is less than clear.






    share|improve this answer











    $endgroup$












    • $begingroup$
      Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
      $endgroup$
      – Henrik Hansen
      1 hour ago















    2












    $begingroup$


    if (data == null || count < 0) yield break;



    This behaviour is somewhat consistent with Take, but not with Skip: Skip treats a negative values as a zero. As, indeed, does the SkipLast which doesn't appear in .NET Framework.



    It should throw on a null argument with an ArgumentNullException.




    My only other real issue with the methods is that neither will work with IEnumerables that can't be enumerated multiple times, and will incur overheads in any that can but have to generate the data lazily.



    I would go for the slightly more painful:





    if (source == null)
    throw new ArgumentNullException("Source Enumeration may not be null", nameof(source));

    if (count <= 0)

    foreach (T item in source)
    yield return item;

    else

    bool yielding = false;
    T[] buffer = new T[count];
    int index = 0;

    foreach (T item in source)

    if (index == count)

    index = 0;
    yielding = true;


    if (yielding)
    yield return buffer[index];

    buffer[index] = item;
    index++;




    If I cared about performance, I might consider the following, which reduces the amount of decision making inside the loop (which might make it faster: I'd better benchmark it).



    // just the bit inside the else
    T[] buffer = new T[count];

    using (var e = source.GetEnumerator())

    // initial filling of buffer
    for (int i = 0; i < buffer.Length; i++)

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

    buffer[i] = e.Current;


    int index = 0;
    while (e.MoveNext())

    yield return buffer[index];
    buffer[index] = e.Current;
    index = (index + 1) % count;






    Performance wise they seems to be even.




    That's encouraging, since Queue<T> is also implemented as a circular buffer. You'd hope that the array based version would be a bit lighter, but may consume more memory is Count > data.Count().



    Having benchmarked your two proposals, my two proposals, and the .NET Core SkipLast (didn't include the Reverse based method), it seems the fastest methods are that built into .NET Core (hurray) and my last one, but the difference between test instance (with different data lengths and skip counts) is great. Unfortunately, I messed up and didn't run a third of the .NET Core tests, so the saga of incompetence on my part continues. The code and data can be found in a gist. The only real conclusion I would want to draw from this data (aside from 'use the BCL method if you can') is that your first method is consistently the slowest when the input array isn't empty in these tests on my machine with it's current workload. The difference is jolly significant, with your first method requiring twice as much time as others in some cases. Why the methods have different performance is less than clear.






    share|improve this answer











    $endgroup$












    • $begingroup$
      Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
      $endgroup$
      – Henrik Hansen
      1 hour ago













    2












    2








    2





    $begingroup$


    if (data == null || count < 0) yield break;



    This behaviour is somewhat consistent with Take, but not with Skip: Skip treats a negative values as a zero. As, indeed, does the SkipLast which doesn't appear in .NET Framework.



    It should throw on a null argument with an ArgumentNullException.




    My only other real issue with the methods is that neither will work with IEnumerables that can't be enumerated multiple times, and will incur overheads in any that can but have to generate the data lazily.



    I would go for the slightly more painful:





    if (source == null)
    throw new ArgumentNullException("Source Enumeration may not be null", nameof(source));

    if (count <= 0)

    foreach (T item in source)
    yield return item;

    else

    bool yielding = false;
    T[] buffer = new T[count];
    int index = 0;

    foreach (T item in source)

    if (index == count)

    index = 0;
    yielding = true;


    if (yielding)
    yield return buffer[index];

    buffer[index] = item;
    index++;




    If I cared about performance, I might consider the following, which reduces the amount of decision making inside the loop (which might make it faster: I'd better benchmark it).



    // just the bit inside the else
    T[] buffer = new T[count];

    using (var e = source.GetEnumerator())

    // initial filling of buffer
    for (int i = 0; i < buffer.Length; i++)

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

    buffer[i] = e.Current;


    int index = 0;
    while (e.MoveNext())

    yield return buffer[index];
    buffer[index] = e.Current;
    index = (index + 1) % count;






    Performance wise they seems to be even.




    That's encouraging, since Queue<T> is also implemented as a circular buffer. You'd hope that the array based version would be a bit lighter, but may consume more memory is Count > data.Count().



    Having benchmarked your two proposals, my two proposals, and the .NET Core SkipLast (didn't include the Reverse based method), it seems the fastest methods are that built into .NET Core (hurray) and my last one, but the difference between test instance (with different data lengths and skip counts) is great. Unfortunately, I messed up and didn't run a third of the .NET Core tests, so the saga of incompetence on my part continues. The code and data can be found in a gist. The only real conclusion I would want to draw from this data (aside from 'use the BCL method if you can') is that your first method is consistently the slowest when the input array isn't empty in these tests on my machine with it's current workload. The difference is jolly significant, with your first method requiring twice as much time as others in some cases. Why the methods have different performance is less than clear.






    share|improve this answer











    $endgroup$




    if (data == null || count < 0) yield break;



    This behaviour is somewhat consistent with Take, but not with Skip: Skip treats a negative values as a zero. As, indeed, does the SkipLast which doesn't appear in .NET Framework.



    It should throw on a null argument with an ArgumentNullException.




    My only other real issue with the methods is that neither will work with IEnumerables that can't be enumerated multiple times, and will incur overheads in any that can but have to generate the data lazily.



    I would go for the slightly more painful:





    if (source == null)
    throw new ArgumentNullException("Source Enumeration may not be null", nameof(source));

    if (count <= 0)

    foreach (T item in source)
    yield return item;

    else

    bool yielding = false;
    T[] buffer = new T[count];
    int index = 0;

    foreach (T item in source)

    if (index == count)

    index = 0;
    yielding = true;


    if (yielding)
    yield return buffer[index];

    buffer[index] = item;
    index++;




    If I cared about performance, I might consider the following, which reduces the amount of decision making inside the loop (which might make it faster: I'd better benchmark it).



    // just the bit inside the else
    T[] buffer = new T[count];

    using (var e = source.GetEnumerator())

    // initial filling of buffer
    for (int i = 0; i < buffer.Length; i++)

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

    buffer[i] = e.Current;


    int index = 0;
    while (e.MoveNext())

    yield return buffer[index];
    buffer[index] = e.Current;
    index = (index + 1) % count;






    Performance wise they seems to be even.




    That's encouraging, since Queue<T> is also implemented as a circular buffer. You'd hope that the array based version would be a bit lighter, but may consume more memory is Count > data.Count().



    Having benchmarked your two proposals, my two proposals, and the .NET Core SkipLast (didn't include the Reverse based method), it seems the fastest methods are that built into .NET Core (hurray) and my last one, but the difference between test instance (with different data lengths and skip counts) is great. Unfortunately, I messed up and didn't run a third of the .NET Core tests, so the saga of incompetence on my part continues. The code and data can be found in a gist. The only real conclusion I would want to draw from this data (aside from 'use the BCL method if you can') is that your first method is consistently the slowest when the input array isn't empty in these tests on my machine with it's current workload. The difference is jolly significant, with your first method requiring twice as much time as others in some cases. Why the methods have different performance is less than clear.







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited 6 hours ago

























    answered 8 hours ago









    VisualMelonVisualMelon

    4,64612 silver badges29 bronze badges




    4,64612 silver badges29 bronze badges











    • $begingroup$
      Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
      $endgroup$
      – Henrik Hansen
      1 hour ago
















    • $begingroup$
      Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
      $endgroup$
      – Henrik Hansen
      1 hour ago















    $begingroup$
    Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
    $endgroup$
    – Henrik Hansen
    1 hour ago




    $begingroup$
    Interesting new optimized versions and tests. I only tested performance on a couple of counts and length and it didn't show much differences. GetEnumerator() has often showed to be much faster than a foreach loop, which IMO is strange, because the latter calls GetEnumerator().
    $endgroup$
    – Henrik Hansen
    1 hour ago













    2












    $begingroup$

    There are 2 cases for which both your and VisualMelon's implementation can be improved:



    • If count is less than or equal to 0, then you can return source directly. This speeds up iteration by removing an unnecessary intermediate step. For this to work, the yielding part of the method has to be moved into another method, but that's easy with local functions.

    • If source is ICollection<T> collection, then you can return source.Take(collection.Count - count), which is faster and uses less memory because it doesn't need to buffer anything. The higher count is, the more of a difference this makes.

    The first of these is probably not a very useful edge-case, so it might not be worth the extra code, but I would definitely include the second optimization.






    share|improve this answer









    $endgroup$












    • $begingroup$
      +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
      $endgroup$
      – VisualMelon
      5 hours ago










    • $begingroup$
      I have actually made a version after my post that is implemented as your first suggestion.
      $endgroup$
      – Henrik Hansen
      1 hour ago















    2












    $begingroup$

    There are 2 cases for which both your and VisualMelon's implementation can be improved:



    • If count is less than or equal to 0, then you can return source directly. This speeds up iteration by removing an unnecessary intermediate step. For this to work, the yielding part of the method has to be moved into another method, but that's easy with local functions.

    • If source is ICollection<T> collection, then you can return source.Take(collection.Count - count), which is faster and uses less memory because it doesn't need to buffer anything. The higher count is, the more of a difference this makes.

    The first of these is probably not a very useful edge-case, so it might not be worth the extra code, but I would definitely include the second optimization.






    share|improve this answer









    $endgroup$












    • $begingroup$
      +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
      $endgroup$
      – VisualMelon
      5 hours ago










    • $begingroup$
      I have actually made a version after my post that is implemented as your first suggestion.
      $endgroup$
      – Henrik Hansen
      1 hour ago













    2












    2








    2





    $begingroup$

    There are 2 cases for which both your and VisualMelon's implementation can be improved:



    • If count is less than or equal to 0, then you can return source directly. This speeds up iteration by removing an unnecessary intermediate step. For this to work, the yielding part of the method has to be moved into another method, but that's easy with local functions.

    • If source is ICollection<T> collection, then you can return source.Take(collection.Count - count), which is faster and uses less memory because it doesn't need to buffer anything. The higher count is, the more of a difference this makes.

    The first of these is probably not a very useful edge-case, so it might not be worth the extra code, but I would definitely include the second optimization.






    share|improve this answer









    $endgroup$



    There are 2 cases for which both your and VisualMelon's implementation can be improved:



    • If count is less than or equal to 0, then you can return source directly. This speeds up iteration by removing an unnecessary intermediate step. For this to work, the yielding part of the method has to be moved into another method, but that's easy with local functions.

    • If source is ICollection<T> collection, then you can return source.Take(collection.Count - count), which is faster and uses less memory because it doesn't need to buffer anything. The higher count is, the more of a difference this makes.

    The first of these is probably not a very useful edge-case, so it might not be worth the extra code, but I would definitely include the second optimization.







    share|improve this answer












    share|improve this answer



    share|improve this answer










    answered 5 hours ago









    Pieter WitvoetPieter Witvoet

    7,1348 silver badges26 bronze badges




    7,1348 silver badges26 bronze badges











    • $begingroup$
      +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
      $endgroup$
      – VisualMelon
      5 hours ago










    • $begingroup$
      I have actually made a version after my post that is implemented as your first suggestion.
      $endgroup$
      – Henrik Hansen
      1 hour ago
















    • $begingroup$
      +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
      $endgroup$
      – VisualMelon
      5 hours ago










    • $begingroup$
      I have actually made a version after my post that is implemented as your first suggestion.
      $endgroup$
      – Henrik Hansen
      1 hour ago















    $begingroup$
    +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
    $endgroup$
    – VisualMelon
    5 hours ago




    $begingroup$
    +1 the BCL implementation does a variation on the former - and the different appears to be measureable - but doesn't for the latter, and there would all benefit from some type specialisation. I suspect I'm going to end up spending a lot of time running benchmarks...
    $endgroup$
    – VisualMelon
    5 hours ago












    $begingroup$
    I have actually made a version after my post that is implemented as your first suggestion.
    $endgroup$
    – Henrik Hansen
    1 hour ago




    $begingroup$
    I have actually made a version after my post that is implemented as your first suggestion.
    $endgroup$
    – Henrik Hansen
    1 hour ago











    0












    $begingroup$

    Takning all considerations by VisualMelon and Pieter Witvoet into account a solution could now be:



    static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)

    if (data == null) throw new ArgumentNullException(nameof(data));
    if (count <= 0) return data;

    if (data is ICollection<T> collection)
    return collection.Take(collection.Count - count);

    IEnumerable<T> Skipper()

    using (var enumer = data.GetEnumerator())

    T[] queue = new T[count];
    int index = 0;

    while (index < count && enumer.MoveNext())
    queue[index++] = enumer.Current;

    index = -1;
    while (enumer.MoveNext())

    index = (index + 1) % count;
    yield return queue[index];
    queue[index] = enumer.Current;




    return Skipper();






    share|improve this answer











    $endgroup$

















      0












      $begingroup$

      Takning all considerations by VisualMelon and Pieter Witvoet into account a solution could now be:



      static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)

      if (data == null) throw new ArgumentNullException(nameof(data));
      if (count <= 0) return data;

      if (data is ICollection<T> collection)
      return collection.Take(collection.Count - count);

      IEnumerable<T> Skipper()

      using (var enumer = data.GetEnumerator())

      T[] queue = new T[count];
      int index = 0;

      while (index < count && enumer.MoveNext())
      queue[index++] = enumer.Current;

      index = -1;
      while (enumer.MoveNext())

      index = (index + 1) % count;
      yield return queue[index];
      queue[index] = enumer.Current;




      return Skipper();






      share|improve this answer











      $endgroup$















        0












        0








        0





        $begingroup$

        Takning all considerations by VisualMelon and Pieter Witvoet into account a solution could now be:



        static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)

        if (data == null) throw new ArgumentNullException(nameof(data));
        if (count <= 0) return data;

        if (data is ICollection<T> collection)
        return collection.Take(collection.Count - count);

        IEnumerable<T> Skipper()

        using (var enumer = data.GetEnumerator())

        T[] queue = new T[count];
        int index = 0;

        while (index < count && enumer.MoveNext())
        queue[index++] = enumer.Current;

        index = -1;
        while (enumer.MoveNext())

        index = (index + 1) % count;
        yield return queue[index];
        queue[index] = enumer.Current;




        return Skipper();






        share|improve this answer











        $endgroup$



        Takning all considerations by VisualMelon and Pieter Witvoet into account a solution could now be:



        static public IEnumerable<T> SkipLast<T>(this IEnumerable<T> data, int count)

        if (data == null) throw new ArgumentNullException(nameof(data));
        if (count <= 0) return data;

        if (data is ICollection<T> collection)
        return collection.Take(collection.Count - count);

        IEnumerable<T> Skipper()

        using (var enumer = data.GetEnumerator())

        T[] queue = new T[count];
        int index = 0;

        while (index < count && enumer.MoveNext())
        queue[index++] = enumer.Current;

        index = -1;
        while (enumer.MoveNext())

        index = (index + 1) % count;
        yield return queue[index];
        queue[index] = enumer.Current;




        return Skipper();







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited 18 mins ago

























        answered 26 mins ago









        Henrik HansenHenrik Hansen

        10.3k1 gold badge13 silver badges35 bronze badges




        10.3k1 gold badge13 silver badges35 bronze badges



























            draft saved

            draft discarded
















































            Thanks for contributing an answer to Code Review Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid


            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.

            Use MathJax to format equations. MathJax reference.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223627%2fskiplast-of-an-ienumerablet-linq-extension%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            Invision Community Contents History See also References External links Navigation menuProprietaryinvisioncommunity.comIPS Community ForumsIPS Community Forumsthis blog entry"License Changes, IP.Board 3.4, and the Future""Interview -- Matt Mecham of Ibforums""CEO Invision Power Board, Matt Mecham Is a Liar, Thief!"IPB License Explanation 1.3, 1.3.1, 2.0, and 2.1ArchivedSecurity Fixes, Updates And Enhancements For IPB 1.3.1Archived"New Demo Accounts - Invision Power Services"the original"New Default Skin"the original"Invision Power Board 3.0.0 and Applications Released"the original"Archived copy"the original"Perpetual licenses being done away with""Release Notes - Invision Power Services""Introducing: IPS Community Suite 4!"Invision Community Release Notes

            Canceling a color specificationRandomly assigning color to Graphics3D objects?Default color for Filling in Mathematica 9Coloring specific elements of sets with a prime modified order in an array plotHow to pick a color differing significantly from the colors already in a given color list?Detection of the text colorColor numbers based on their valueCan color schemes for use with ColorData include opacity specification?My dynamic color schemes

            Tom Holland Mục lục Đầu đời và giáo dục | Sự nghiệp | Cuộc sống cá nhân | Phim tham gia | Giải thưởng và đề cử | Chú thích | Liên kết ngoài | Trình đơn chuyển hướngProfile“Person Details for Thomas Stanley Holland, "England and Wales Birth Registration Index, 1837-2008" — FamilySearch.org”"Meet Tom Holland... the 16-year-old star of The Impossible""Schoolboy actor Tom Holland finds himself in Oscar contention for role in tsunami drama"“Naomi Watts on the Prince William and Harry's reaction to her film about the late Princess Diana”lưu trữ"Holland and Pflueger Are West End's Two New 'Billy Elliots'""I'm so envious of my son, the movie star! British writer Dominic Holland's spent 20 years trying to crack Hollywood - but he's been beaten to it by a very unlikely rival"“Richard and Margaret Povey of Jersey, Channel Islands, UK: Information about Thomas Stanley Holland”"Tom Holland to play Billy Elliot""New Billy Elliot leaving the garage"Billy Elliot the Musical - Tom Holland - Billy"A Tale of four Billys: Tom Holland""The Feel Good Factor""Thames Christian College schoolboys join Myleene Klass for The Feelgood Factor""Government launches £600,000 arts bursaries pilot""BILLY's Chapman, Holland, Gardner & Jackson-Keen Visit Prime Minister""Elton John 'blown away' by Billy Elliot fifth birthday" (video with John's interview and fragments of Holland's performance)"First News interviews Arrietty's Tom Holland"“33rd Critics' Circle Film Awards winners”“National Board of Review Current Awards”Bản gốc"Ron Howard Whaling Tale 'In The Heart Of The Sea' Casts Tom Holland"“'Spider-Man' Finds Tom Holland to Star as New Web-Slinger”lưu trữ“Captain America: Civil War (2016)”“Film Review: ‘Captain America: Civil War’”lưu trữ“‘Captain America: Civil War’ review: Choose your own avenger”lưu trữ“The Lost City of Z reviews”“Sony Pictures and Marvel Studios Find Their 'Spider-Man' Star and Director”“‘Mary Magdalene’, ‘Current War’ & ‘Wind River’ Get 2017 Release Dates From Weinstein”“Lionsgate Unleashing Daisy Ridley & Tom Holland Starrer ‘Chaos Walking’ In Cannes”“PTA's 'Master' Leads Chicago Film Critics Nominations, UPDATED: Houston and Indiana Critics Nominations”“Nominaciones Goya 2013 Telecinco Cinema – ENG”“Jameson Empire Film Awards: Martin Freeman wins best actor for performance in The Hobbit”“34th Annual Young Artist Awards”Bản gốc“Teen Choice Awards 2016—Captain America: Civil War Leads Second Wave of Nominations”“BAFTA Film Award Nominations: ‘La La Land’ Leads Race”“Saturn Awards Nominations 2017: 'Rogue One,' 'Walking Dead' Lead”Tom HollandTom HollandTom HollandTom Hollandmedia.gettyimages.comWorldCat Identities300279794no20130442900000 0004 0355 42791085670554170004732cb16706349t(data)XX5557367