-4

In C#, what is the most efficient way to return the last N elements of an array?

I am dealing with arrays of hundreds of thousands of elements, and I would prefer an answer that is more efficient than a LINQ based solution.

I would also prefer that the answer is unit tested, to avoid out-by-one errors.

Contango
  • 76,540
  • 58
  • 260
  • 305
  • 1
    What have you tried to do to solve this problem, and what problem(s) have you had with your attempted solutions? – Servy Sep 09 '15 at 15:19
  • 1
    This is a "share your knowledge, Q&A-style" style of entry. – Contango Sep 09 '15 at 15:21
  • 4
    That doesn't in any way lower the quality standards for questions or answers. The question should be a quality question regardless of whether or not you post an answer, and the answer needs to be up to the standards of the site even if you're the one that asked the question. – Servy Sep 09 '15 at 15:22
  • 1
    Stop editing meta information into the question. The question is where you ask your question, not where you explain why you're asking your question. – Servy Sep 09 '15 at 15:26
  • This is not a duplicate question. It's a more efficient implementation for an array. I'm dealing with arrays of hundreds of thousands of elements, and the previous implementation is very slow. – Contango Sep 09 '15 at 15:28
  • @Contango then answer that question with your new implementation. – Dave Zych Sep 09 '15 at 15:28
  • There are some existing question on this topic which can be found by searching https://www.bing.com/search?q=Get+the+last+N+elements+of+an+array+in+C%23 - it makes this question either duplicate OR very low quality due to lack of demonstrated research. Please consider which option matches your intention and update post accordingly. At this point look like duplicate to me. – Alexei Levenkov Sep 09 '15 at 15:29
  • @Contango The accepted answer is very slow; there are other answers to that question that are notably more efficient than your code. Dave is right that you could add your answer if it would add value, but that's not necessary as there are already answers there that use a `for` loop over an `IList`. – Servy Sep 09 '15 at 15:31
  • 1
    These answers use LINQ and IEnumerable<>, and do not use arrays. The original question specifically asks for a LINQ based solution, which disallows any array based solutions. – Contango Sep 09 '15 at 15:33
  • @Contango *Some*, but not *all* of those answers are based on `IEnumerable` inputs. – Servy Sep 09 '15 at 15:34
  • To add to @Servy's point http://stackoverflow.com/questions/3453274/using-linq-to-get-the-last-n-elements-of-a-collection is more generic than your answer and includes tests already. – Alexei Levenkov Sep 09 '15 at 15:38
  • **Can we have some evidence of efficiency supplied with each answer please.** Feel free to prove that you have unlocked massive performance gains over and above the [List.GetRange Method](https://msdn.microsoft.com/en-us/library/21k0e39c%28v=vs.110%29.aspx) – Nathan Cooper Sep 09 '15 at 15:48
  • I think this is a valid question. It specifically asks for an array implementation. And although the answers in the "duplicate" question would work on array too, they would not perform very well on large arrays which the questions seems to be about. – Magnus Sep 09 '15 at 16:09
  • @Magnus Except for all of the ones that would, which is around half of them. You need to look past just the accepted answer. There are multiple answers using a very similar approach to what the OP is doing, several that will even perform better. – Servy Sep 09 '15 at 16:18
  • @Servy Which one(s) are you referring to that would perform better? – Magnus Sep 09 '15 at 16:27
  • @Magnus [Here](http://stackoverflow.com/a/28372263/1159478) is one, although there are like 4 that act on lists and thus avoid iterating the early items in the collection. – Servy Sep 09 '15 at 16:30
  • @Servy I disagree, but on the other hand you usually know what you are talking about. – Magnus Sep 09 '15 at 16:33
  • @Magnus In terms of performance, the main win is adding deferred execution. It's true that at the end of the day you can't get better than just iterating through all N items in the result set, at least if the entire result set is actually used. – Servy Sep 09 '15 at 16:35
  • 1
    @Servy In my use case, I wanted the last 10 items from an array of 100,000 elements. Yes, you can get better performance than just iterating through all N elements in the result set, if the array supports indexes (which all arrays do). – Contango Sep 09 '15 at 23:04
  • @Contango No, you can't. You expect to be able to use the 10 items from the result set without actually iterating through the 10 items in the result set, because the array supports being indexed? That makes no sense at all. You don't need to iterate through all items in the *source* set, which is something that the duplicate question has multiple answers that avoid, in ways that are even better than your answer. – Servy Sep 10 '15 at 12:29

2 Answers2

1

I'd probably write:

public static T[] TakeLast<T>(this T[] source, int n)
{
        if(source == null)
            throw new ArgumentNullException(nameof(source));
        if(n > source.Length)
            throw new ArgumentOutOfRangeException(nameof(n), "Can't be bigger than the array");
        if(n < 0)
            throw new ArgumentOutOfRangeException(nameof(n), "Can't be negative");

        var target = new T[n]; 
        Array.Copy(source, source.Length - n, target, 0, n);
        return target;
}
Magnus
  • 45,362
  • 8
  • 80
  • 118
-1

As follows. Unit tests are for NUnit.

[TestFixture]
public static class MyTakeLastExtensions
{   
    /// <summary>
    /// Intent: Returns the last N elements from an array.
    /// </summary>
    public static T[] MyTakeLast<T>(this T[] source, int n)
    {
        if (source == null)
        {
            throw new Exception("Source cannot be null.");
        }
        if (n < 0)
        {
            throw new Exception("Index must be positive.");
        }
        if (source.Length < n)
        {
            return source;
        }
        var result = new T[n];
        int c = 0;
        for (int i = source.Length - n; i < source.Length; i++)
        {
            result[c] = source[i];
            c++;
        }
        return result;
    }

    [Test]
    public static void MyTakeLast_Test()
    {
        int[] a = new[] {0, 1, 2};
        {
            var b = a.MyTakeLast(2);
            Assert.True(b.Length == 2);
            Assert.True(b[0] == 1);
            Assert.True(b[1] == 2);
        }

        {
            var b = a.MyTakeLast(3);
            Assert.True(b.Length == 3);
            Assert.True(b[0] == 0);
            Assert.True(b[1] == 1);
            Assert.True(b[2] == 2);
        }

        {
            var b = a.MyTakeLast(4);
            Assert.True(b.Length == 3);
            Assert.True(b[0] == 0);
            Assert.True(b[1] == 1);
            Assert.True(b[2] == 2);
        }

        {
            var b = a.MyTakeLast(1);
            Assert.True(b.Length == 1);
            Assert.True(b[0] == 2);
        }

        {
            var b = a.MyTakeLast(0);
            Assert.True(b.Length == 0);
        }

        {               
            Assert.Throws<Exception>(() => a.MyTakeLast(-1));
        }

        {
            int[] b = null;
            Assert.Throws<Exception>(() => b.MyTakeLast(-1));
        }
    }
}
Dave Zych
  • 21,581
  • 7
  • 51
  • 66
Contango
  • 76,540
  • 58
  • 260
  • 305
  • If you really want to return source in case of `source.Length < n`, why not when `source.Length <= n`? – Henrik Sep 09 '15 at 15:32
  • 4
    But IMHO it's a bad idea, to sometimes return the original array and sometimes a new one. – Henrik Sep 09 '15 at 15:35
  • 1
    As @Henrik pointed out it is strange that you only copy collection sometimes - this will lead to random behavior when origin collection is changed. If you are fine with just using origin collection why even make a copy - basic `yield return` (or custom iterator) would be more memory efficient. – Alexei Levenkov Sep 09 '15 at 15:42