29

I have the following code:

newsplit.ToList().ForEach(x => x = "WW");

I would expect that all elements in the list are now "WW" but they are still the original value. How come? What do I have to do different?

Jon
  • 38,814
  • 81
  • 233
  • 382

5 Answers5

41

Assuming that newsplit is an IEnumerable<string>, you want:

newsplit = newsplit.Select(x => "WW");

The code that you currently have is equivalent to the following:

foreach(string x in newsplit.ToList()) {
    AssignmentAction(x);
}

...

public static void AssignmentAction(string x) {
    x = "WW";
}

This method won't modify x because of the pass-by-value semantics of C# and the immutability of strings.

jason
  • 236,483
  • 35
  • 423
  • 525
  • Is a string array or in my case a List IEnumerable as I dont think it is? – Jon Jul 21 '09 at 16:28
  • @Jon: arrays and List all support the IEnumerable interface – Jeff Yates Jul 21 '09 at 16:31
  • 1
    I don't think a Select() is what Jon wants, it sounds like he wants to modify the IEnumerable itself. Select will just give him the items that already are "WW" – AgileJon Jul 21 '09 at 16:32
  • @AgileJon: It sounds like you are confusing IEnumerable.Where and IEnumerable.Select. – jason Jul 21 '09 at 16:38
  • @Downvoters: Two downvotes? Is this wrong? Is this harmful? What am I overlooking? – jason Jul 21 '09 at 17:08
  • @Jason: you are right, I was confusing Select with Where. Downvote removed. – AgileJon Jul 21 '09 at 17:26
  • newSplit is a list of items, in this example it is a string but I also have a List. I want to change each item in the List. However List test = newsplit.Select(x => x = "WW"); doesn't work. Does it need to be List test = newSplit.ToList().Select(x => x = "WW"); ?? – Jon Jul 21 '09 at 18:13
  • @Jon: If you want to create a new List with the altered values you could say List test = newsplit.Select(x => "WW").ToList(). – jason Jul 21 '09 at 18:46
  • What if you already have a list and want to change its values, surely calling Select.ToList() is a bit odd as its already a list? – Jon Jul 22 '09 at 08:15
20

Other answers have explained why your current code doesn't work. Here's an extension method which would fix it:

// Must be in a static non-nested class
public static void ModifyEach<T>(this IList<T> source,
                                 Func<T,T> projection)
{
    for (int i = 0; i < source.Count; i++)
    {
        source[i] = projection(source[i]);
    }
}

Then use like this:

newsplit.ModifyEach(x => "WW");

That will work with any implementation of IList<T> such as arrays and List<T>. If you need it to work with an arbitrary IEnumerable<T> then you've got a problem, as the sequence itself may not be mutable.

Using Select() is a more functional approach of course, but sometimes mutating an existing collection is worth doing...

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    @Jon: I think you meant newsplit.ToList().ModifyEach(x => "WW") in your second code block? – jason Jul 21 '09 at 16:52
  • 2
    @Jason: Nearly... but if you call ToList() but don't keep a reference to that new list, the modifications won't be preserved. In particular it would *not* modify the original collection. Have edited answer for intended use. – Jon Skeet Jul 21 '09 at 17:14
4

The ForEach will allow you to manipulate the elements of the IEnumerable, but not change the reference of the element.

ie, this would set a Foo property of each element in the IEnumerable to the string "WW":

newsplit.ToList().ForEach(x => x.Foo = "WW");

However, you won't be able to modify the values inside the IEnumerable itself.

AgileJon
  • 53,070
  • 5
  • 41
  • 38
  • At least in VB.Net I could only assign property of element inside ForEach if the multi-line ForEach, the Linq ForEach wouldn't assign by reference. – Brent Jun 17 '15 at 17:32
2

It's because the LINQ expression is creating Anonymous Types, and these are read-only. They can't be assigned to. Also, in a standard for each loop you cannot assign to a collection that is being enumerated. Try:

    foreach (var item in newsplit)
    {
        item = "this won't work";
    }
David Refoua
  • 3,476
  • 3
  • 31
  • 55
Dan Diplo
  • 25,076
  • 4
  • 67
  • 89
  • There is no anonymous type created in the given ForEach expression. The parameter is a delegate, which is applied to each element in the iteration; and of course, won't work for the reason you give. – Steve Guidi Jul 21 '09 at 16:44
  • You are right, of course - not sure what I was thinking! But the second part, about collections being immutable when iterated, is worth noting. Thanks. – Dan Diplo Jul 21 '09 at 20:24
1

You can write like this:

newsplit = newsplit.Select(x => "WW").ToList();
Wang_TWJ
  • 63
  • 9