31

I'm trying to find the LINQ equivalent of the following code:

NameValueCollection nvc = new NameValueCollection();

List<BusinessLogic.Donation> donations = new List<BusinessLogic.Donation>();
donations.Add(new BusinessLogic.Donation(0, "", "", "");
donations.Add(new BusinessLogic.Donation(0, "", "", "");
donations.Add(new BusinessLogic.Donation(0, "", "", "");

for(var i = 0; i < donations.Count(); i++)
{
    // NOTE: item_number_ + i - I need to be able to do this
    nvc.Add("item_number_" + i, donations[i].AccountName);
}

I was hoping I could use something like:

NameValueCollection nvc = new NameValueCollection();

List<BusinessLogic.Donation> donations = new List<BusinessLogic.Donation>();
donations.Add(new BusinessLogic.Donation(0, "", "", "");
donations.Add(new BusinessLogic.Donation(0, "", "", "");
donations.Add(new BusinessLogic.Donation(0, "", "", "");

donations.ForEach(x => nvc.Add("item_name_" + ??, x.AccountName);

But I've not found a way to determine which iteration the loop is on. Any help would be appreciated!

James Hill
  • 60,353
  • 20
  • 145
  • 161

12 Answers12

37

LINQ doesn't have a ForEach method, and for good reason. LINQ is for performing queries. It is designed to get information from some data source. It is not designed to mutate data sources. LINQ queries shouldn't cause side effects, which is exactly what you're doing here.

The List class does have a ForEach method, which is what you are using. Because it's not actually in the System.Linq namespace it's not technically a part of LINQ.

There is nothing wrong with the for loop in your question. It would be wrong (from a good practice perspective) to try to change it in the way that you're trying to.

Here is a link that discusses the matter in more detail.

Now, if you want to ignore that advice and use a ForEach method anyway, it's not hard to write one that provides an index to the action:

public static void ForEach<T>(this IEnumerable<T> sequence, Action<int, T> action)
{
    // argument null checking omitted
    int i = 0;
    foreach (T item in sequence)
    {
        action(i, item);
        i++;
    }
}
Servy
  • 202,030
  • 26
  • 332
  • 449
  • Well, I've obviously used the wrong terminology. My collection of `Donation` does have a `.ForEach()` method. – James Hill Oct 24 '12 at 17:23
  • ForEach isn't defined as an extension method of IEnumerable and therefore isn't part of LINQ. – devdigital Oct 24 '12 at 17:24
  • 6
    @JamesHill It's a method of `List`. It's not a method in `System.Linq` – Servy Oct 24 '12 at 17:24
  • 3
    @Servy, I think it's obvious that I need to step away from my computer for a while. I think coffee is in order. Thanks for setting me straight. – James Hill Oct 24 '12 at 17:30
  • 2
    +1 for the link to that article. I've wondered why there wasn't a LINQ implementation of ForEach, but that makes perfect sense. – JDB Oct 24 '12 at 17:32
  • I like this, although you can drop the index computations and shorten up the code by replacing the implementation with just `foreach (var e in sequence.Select((item,n)=>(item,n))) action(e.n, e.item);` – Jason C Mar 05 '22 at 21:16
25

If you really want to use a List.ForEach, it's simple:

//[...]
int i=0;
donations.ForEach(x => nvc.Add("item_name_" + i++, x.AccountName);
Tiago Raposo
  • 259
  • 3
  • 2
15

It's a little convoluted and creates an intermediate collection, but how about:

donations.Select((x, i) => new {Name = "item_name_" + i, x.AccountName})
    .ToList()
    .ForEach(x=> nvc.Add(x.Name, x.AccountName));

This uses the overload of Enumerable.Select which incorporates the index.

I do have to argue there is nothing really to gain from doing it this way. You create more overhead with the intermediate collection and IMHO lose readability over your original for-loop.

You can also skip the intermediate collection if you're willing to use foreach loop instead of List.ForEach. See @wageoghe's answer (again highly recommended).

lc.
  • 113,939
  • 20
  • 158
  • 187
14

This is an old post but is highly ranked by Google so I figured a more generic approach would be suitable. (Also, I tend to forget how this is done which means I have to google every time...)

Assume a list of integers:

var values = new List<int>() { 2, 3, 4, 0x100, 74, 0xFFFF, 0x0F0F };

To iterate the list and having an index do the following:

values.Select((x, i) => new
{
    item = x,
    index = i
})
.ToList()
.ForEach(obj =>
{
    int value = obj.item;
    int valueIndex = obj.index;
});
Patrick Ribbing
  • 207
  • 2
  • 7
  • The downside to this is you lose lazy evaluation of `values`, as `ToList()` will force an enumeration over the entire source before `ForEach` is applied. E.g. see the difference in the output of the two methods [in this program](https://dotnetfiddle.net/Gikaox). – Jason C Mar 05 '22 at 21:24
  • I'm not a developer, and sometimes I get mad when devs gives answers like "This is not how you should do it, because blah blah blah...." And I understand there's always a better way. But it's answers like yours that make me want to learn more about programming. Sometimes is not about the best way. Thank you! List input; byte[] bytedata = new byte[input.Count]; input.Select((x, i) => new { item = x, index = i }).ToList().ForEach(o => bytedata[o.index] = (byte)o.item); – FranciscoNabas Oct 24 '22 at 15:46
  • And tell you what, made a simple raw test, and your method was faster than using a for loop. – FranciscoNabas Oct 24 '22 at 15:56
4

Piggybacking on the answer by @lc.

foreach (var x in donations.Select((d, i) => new {ItemName = "item_name_" + i, AccountName = d.AccountName}))
{
  nvc.Add(x.ItemName, x.AccountName);
}
wageoghe
  • 27,390
  • 13
  • 88
  • 116
  • Now that tuples exist you can shorten up the `new{}` syntax a bit by using `(d, i) => ($"item_name_{i}", d.AccountName)` instead. Then either use `x.Item1` and `x.Item2`, or name the tuple fields instead of using `x`. – Jason C Mar 05 '22 at 21:26
3

Is there any reason you're not using a Dictionary<string, string> as your names/keys appear to be unique? This would be faster and you could use the ToDictionary standard query operator.

Also, if you did wish to use an extension method (although as Servy says a for loop is the right solution here), then you could write your own - see here.

Community
  • 1
  • 1
devdigital
  • 34,151
  • 9
  • 98
  • 120
3

This can be done using aggregate as well. Check the following example:

var data = new[]
{
    "A",
    "B",
    "C",
    "D"
};

var count = data.Aggregate(0, (index, item) =>
{
    Console.WriteLine("[{0}] => '{1}'", index, item);

    return index + 1;
});

Console.WriteLine("Total items: {0}", count);

Aggregate here acts as a for statement. The only downside is that you need to increase the value of the index by one in each iteration and return it.

Soroush Falahati
  • 2,196
  • 1
  • 27
  • 38
0

I like to do that the following way:

NameValueCollection nvc = new NameValueCollection();

List<BusinessLogic.Donation> donations = new List<BusinessLogic.Donation>();
donations.Add(new BusinessLogic.Donation(0, "", "", ""));
donations.Add(new BusinessLogic.Donation(0, "", "", ""));
donations.Add(new BusinessLogic.Donation(0, "", "", ""));

Enumerable
    .Range(0, donations.Count())
    .ToList()
    .ForEach(i => nvc.Add("item_number_" + i, donations[i].AccountName));
an.dr.eas.k
  • 151
  • 8
-1

Try this -

donations.ForEach(x =>
         {
             int index = donations.IndexOf(x);
             nvc.Add("item_name_" + index, x.AccountName);
         });
Rohit Vats
  • 79,502
  • 12
  • 161
  • 185
  • 6
    You're better off enumerating an intermediate collection with indices (a la @lc's answer) than searching the list on every iteration (`.IndexOf`) – Austin Salonen Oct 24 '12 at 17:35
  • 3
    What would happen if an item was in the list multiple times? (Note that in the OP's code all of the items are identical.) It's not *just* about the terrible performance after all; it doesn't even *work*. – Servy Oct 24 '12 at 17:37
  • So, is there any another way of finding index w/o creating an intermediate collection..? – Rohit Vats Oct 24 '12 at 17:38
  • 2
    @Sevy - Oops..!! Yeah it makes sense now to me. Just completely forget the uniqueness part. Downvotes accepted with honour now. :) – Rohit Vats Oct 24 '12 at 17:40
  • @RV1987 Sure. Use a `for` loop like is done in the OP's code. Or you could make a `ForEach` method that provides an index, like my answer shows, or you could create an integer `count` and increment it yourself in the loop. – Servy Oct 24 '12 at 17:40
  • 1
    @Sevy - Yeah thanks Servy got the point now..That's why i said atleast tell me where i am wrong. – Rohit Vats Oct 24 '12 at 17:41
-2

C# 7 (circa 2017) added a cleaner tuple syntax, which can be used with the indexed form of Select (there's no need to compute the index manually) to give a nice concise syntax. E.g.:

foreach ((var item, int n) in TheItems.Select((i,n)=>(i,n))) {
    // item is the current item, n is its index
}

Or if you prefer:

foreach (var element in TheItems.Select((item,n)=>(item,n))) {
    // element.item is the current item, element.n is its index
}

So in the OP's case, for example:

foreach ((var item, int n) in donations.Select((i,n)=>(i,n)))
    nvc.Add($"item_number{n}", item.AccountName);

Or, if you prefer:

foreach ((var k, var v) in donations.Select((i,n)=>($"item_number_{n}",i.AccountName)))
    nvc.Add(k, v);

Or, similar to this answer but slightly less verbose, but note that this will kill lazy evaluation by calling ToList(), which forces enumeration over the whole source prior to calling ForEach():

TheItems.Select((item,n)=>(item,n)).ToList().ForEach(element => { 
    // element.item is the current item, element.n is its index
});
Jason C
  • 38,729
  • 14
  • 126
  • 182
  • *I feel like this solution already should have been here* -- Well, there are *several* answers based on this `Select` overload. What's new here? – Gert Arnold Mar 05 '22 at 11:25
  • @GertArnold foreach over Select, and not maintaining an index counter, and more concise code. Most of the answers "based on" Select do a bit of extra stuff or kill the lazy evaluation. – Jason C Mar 05 '22 at 20:56
  • The only thing that matters here is the `Select` overload. You make it seem you're the first to mention it. Even then, there's really no need to add an endless number of varieties how to use it. In the mean time one might wonder why no one has yet simply created `nvc` from `donations` directly only using `Select` and `ToList` without `Foreach` or `foreach`. That would have been the least verbose solution. I don't feel any urge to add yet another answer though. – Gert Arnold Mar 06 '22 at 10:23
  • @GertArnold In the general sense (although a non-issue in the OP's case), using `ToList()` will force enumeration over the entire source series before proceeding. So if the source is some lazily produced IEnumerable, you'll lose the lazy evaluation there, which can be a problem in certain situations. – Jason C Mar 06 '22 at 19:07
  • @GertArnold Re: Creating `nvc` directly, [an.dreas.k's answer](https://stackoverflow.com/a/53429193) is pretty close, but there aren't really any methods to produce a `NameValueCollection` like that. `ToDictionary()` doesn't help (but [devdigital](https://stackoverflow.com/a/13054522)'s suggestion would make it possible), and while you could build an NVC with e.g. `Aggregate` or something, at that point you might as well just use `Add`. Also it's unclear if the OP intends to create a new NVC or add to an existing one, but from the question it looks slightly more like the latter. – Jason C Mar 06 '22 at 19:07
  • @GertArnold Btw, there is only one other answer here that 1) doesn't explicitly compute the index, and 2) preserves lazy evaluation. My main point in this answer was more that tuples (which didn't exist in their current form when the older answers were posted) clean up the syntax even further. I do see what you mean about "You make it seem you're the first to mention [Select]"; I can reword that. – Jason C Mar 06 '22 at 19:15
-2

I strongly recommend against this method, unless you're trolling your friends.

This is a way to create a "For" loop with Linq, but with MANY downsides:

int[] arr = new int[] {1, 1, 1, 1};

Enumerable.Range(0, arr.length)
    .ToList()
    .ForEach(i => 
        {
            arr[i] += 2;
        });

The array should now be {3, 3, 3, 3}.

Again, do not use this!

Some of these downsides are (but not limited to):

  • Really slow performance
  • Large amount of memory usage
  • Unreadable

Here's an example of how unreadable it can be: Unreadable example

I made this so my friend wouldn't mess with my code.

The code creates a "for" loop that goes from 0.00 to 1.00 at an 0.01 increment and then "bakes" the curve.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
BOT Alex
  • 37
  • 2
-3

A crappy solution would be to simply use select and return a useless value.

items.toList().Select((el, index) =>
{
    el.doStuff();
    return 0;
});
John
  • 5,942
  • 3
  • 42
  • 79
  • 1
    I just did this. Don't tell anybody, lol. – Jason C Mar 05 '22 at 03:59
  • 1
    @JasonC Your secret is safe with me – John Mar 05 '22 at 04:01
  • This code has almost as many issues as words. It does nothing because the statement is not executed. And when spelling is corrected, `ToList` is redundant. The `Select` body doesn't do anything with `index`. When executed, it would only return a list of zeros and (probably) change the source objects, which isn't asked. It won't add anything to `NameValueCollection nvc`. Crappy, sure. – Gert Arnold Mar 05 '22 at 16:49
  • 2
    @GertArnold its pseudo code to convey the concept. Not a pastable sample. If you're going to complain about spelling, you should know that sentences do not start with "And". `ToList` is not redundant since it implies that items was an standard type array that needed conversion. It doesn't need to do anything with index, its just showing that its available if you need it. `foreach` doesn't need to return anything in the first place so I don't know why you're whining about that. – John Mar 05 '22 at 17:29
  • Pseudo code isn't really useful in answers. Besides, there were already enough answers suggesting this overload of `Select`. And telling how to use it in real code. – Gert Arnold Mar 05 '22 at 17:39
  • @GertArnold Even if ToList were redundant, nobody would force you to not type it. Also, pseudo code is extremely useful in examples although, yes, you'd need a reasonably functional neural network with basic reasoning capabilities to apply the concept, which may or may not be an issue for you. – Jason C Mar 05 '22 at 20:00
  • Also the key and unique point of this answer is using Select directly to execute the action. The unused return value is the whole point, it's the key part of this kludge. To use it in real code just ... use it in real code. – Jason C Mar 05 '22 at 20:03
  • @GertArnold Here you go buddy, this one's for you: `Parallel.Invoke(new ParallelOptions { MaxDegreeOfParallelism = 1 }, Data.Select((e,n) => () => DoSomething(e,n)).ToArray());`, and it doesn't even waste any returned values. https://dotnetfiddle.net/chd5i7 – Jason C Mar 05 '22 at 22:05