0

I have the following list: 

List<int> listOfInt = new List<int> {10, 20, 30, 40, 50};

I am now given a list of indices pointing to the values to be extracted from listOfInt: 

int[] idxList= new int[] { 2, 4, 1 };

Now, I want to extract all the values from listOfInt by index (as per provided idxList) into another list.
Here is what I came up with, which seems to work, but I would like to get rid of this foreach loop. How can this be done? 

List<int> newList= new List<int>();
foreach(var idx in idxList) newList.Add( listOfInt.ElementAt(idx) );

Solution:
{30, 50, 20} will be written into newList

stackMeUp
  • 522
  • 4
  • 16
  • 2
    You said "efficient", but it seems you are looking for simpler/better looking code, not code with better performance, right? – Sohaib Jundi Jul 11 '19 at 08:23
  • Well, it depends, can we do more efficient without the foreach loop? – stackMeUp Jul 11 '19 at 08:25
  • 2
    Something has to loop whether you like it or not, since that is the case, now we are down to micro-optimization, i suggest getting a benchmarker and trying a couple of things – TheGeneral Jul 11 '19 at 08:26
  • Ok thanks, are you saying my solution is already optimal? – stackMeUp Jul 11 '19 at 08:27
  • 2
    This question might be better suited for https://codereview.stackexchange.com/ than in here, but check their site guidelines – Andreas Jul 11 '19 at 08:27
  • Thanks Andreas, I will check it out! – stackMeUp Jul 11 '19 at 08:28
  • "but I would like to get rid of this foreach loop" why? It works as expected. If you're trying to optimise this, then it feels like premature optimisation. – phuzi Jul 11 '19 at 08:29
  • I guess it is a tradeoff. Using Linq seems to be the alternative. I had the feeling I was not coding as a C# programmer when using a foreach loop, but rather like a C programmer. Hence not using all the power provided by the C# language. – stackMeUp Jul 11 '19 at 08:36

4 Answers4

3

Well, it depends, can we do more efficient without the foreach loop?

foreach() gets an enumerator and walks over the list. Any other solution you can come up with, will ultimately have to do the same, because you need to handle each item in your index list.

So no, you can't make this more efficient. Or maybe you could, if you write out all requirements and assumptions, but no way this is going to be the performance bottleneck of your code.

CPU cycles are cheap. Terse code is expensive, to both write and read, because of the mental overhead.

You could use Linq as @Sohaib demonstrates below, but that won't be noticeably more efficient.

CodeCaster
  • 147,647
  • 23
  • 218
  • 272
  • I appreciate the tips, thanks. I guess I need to become confortable with both approaches (efficient and readable). – stackMeUp Jul 11 '19 at 08:33
1

Short way to do this, using Linq: idxList.Select(i => listOfInt[i]).ToList()

Sohaib Jundi
  • 1,576
  • 2
  • 7
  • 15
  • Oh, I like that. Nice one! How does it compare to using the foreach loop? Is it more/less/same type of efficiency? – stackMeUp Jul 11 '19 at 08:31
  • 1
    @stackMeUp the source code for Select uses a foreach – TheGeneral Jul 11 '19 at 08:32
  • this is just a short hand for it, as far as I know, using linq imposes some overhead – Sohaib Jundi Jul 11 '19 at 08:33
  • @stackMeUp I recommend you read the answer of [this](https://stackoverflow.com/questions/34995553/whats-the-difference-between-select-new-vs-foreach-new) question. – Ammar Jul 11 '19 at 08:35
  • +1 @Sohaib Jundi: What I like about your solution is that I can return the entire result into a variable. When using a foreach loop, newList will have to be cleared each time I enter the loop. – stackMeUp Jul 11 '19 at 08:42
1

If you want to omit foreach you can use this:

var result = idxList.Select(i => listOfInt.ElementAt(i));
Saeid Amini
  • 1,313
  • 5
  • 16
  • 26
0

You are adding items to newList one by one. This will create some unnecessary memory allocations. Better to give new List hint about resulting size.

var listOfInt = new List<int> { 10, 20, 30, 40, 50 };
var idxList = new [] { 2, 4, 1 };

var newList = new List<int>(idxList.Length);

for (int i = 0; i < idxList.Length; i++)
    newList[i] = listOfInt[idxList[i]];

Also, for loop tends to be faster than foreach when iterating thru List (source).

Matěj Pokorný
  • 16,977
  • 5
  • 39
  • 48
  • Yes that is what I realised and why using Linq as suggested by @Sohaib Jundi seems to be the way to go. Thanks. – stackMeUp Jul 11 '19 at 09:24