What's Wrong With This Code? (#14)

Tuesday, April 17, 2007

MSDN says the List.ForEach method will "perform the specified action on each element of the List".

The following program does print out a haiku to the console, but does not make the haiku all lowercase.

using System;
using System.Collections.Generic;

class WWWTC
{
    
static void Main()
    {
        
List<string> haiku = new List<string>();

        haiku.Add(
"All software changes");
        haiku.Add(
"Upon further reflection -");
        haiku.Add(
"A reference to String");

        
// make it lowercase
        haiku.ForEach(
                    
delegate(string s)
                    {
                        s = s.ToLower();
                    }
        );

        
// ... and now print it out
        haiku.ForEach(
                    
delegate(string s)
                    {
                        
Console.WriteLine(s);
                    }
        );
    }
}

 What's wrong?  


Comments
Haacked Tuesday, April 17, 2007
The string s passed into the anonymous delegate is not being passed by reference, so you're setting a local scoped variable to a lowercase version and not affecting the original member of the list.
zproxy Tuesday, April 17, 2007
Obviously the string is passed by value thus not saved after the action is invoked.
Eber Irigoyen Tuesday, April 17, 2007
There's nothing wrong with the code, is doing exactly what you told it to do =oP

...strings are immutable
Eber Irigoyen Tuesday, April 17, 2007
in the case of strings, the parameter is like a local copy of the value, and so when you change it you are changing your local copy (inside the delegate), a reference parameter would be needed in that case, but is not compatible with the required delegate for the ForEach, the solution is to iterate through the list using the index

// make it lowercase
for (int i = 0; i < haiku.Count; i++)
haiku[i] = haiku[i].ToLower();
William Tuesday, April 17, 2007
The line s = s.ToLower() is merely changing the location that the local reference s is pointing to.

Changing the local reference s will not change the original pointer haiku[x].
James H Tuesday, April 17, 2007
As everyone has already pointed out what's wrong, time to point out how to fix it :)

You want ConvertAll, rather than ForEach. Something like:

haiku = haiku.Convertall<string>(
delegate(string s)
{
return s.ToLower();
}
);
Jonathan Bates Tuesday, April 17, 2007
Change this line:
s = s.ToLower();

to this line:
haiku[haiku.IndexOf(s)] = s.ToLower();
Eric D. Burdo Tuesday, April 17, 2007
WOOHOO! I understood one of the code problems! *dance* *dance*

Oh yeah.. the answer. Same as others have said. Strings are immutable. Since your using a local copy of s in each block, they don't see the changes made by the other block.
Dustin Campbell Tuesday, April 17, 2007
I'm surprised at the comments that seem to think that this is caused by the fact that strings are immutable. Truthfully, the code presented wouldn't work even if they were mutable. The problem (as noted by Haaked and zproxy) is that the parameter is passed by value and not by reference.

You could define your own MutableAction delegate and corresponding MutableForEach to allow passing by reference.

Solution #1:

using System;
using System.Collections.Generic;

class WWWTC
{
delegate void MutableAction<T>(ref T item);

static void MutableForEach<T>(List<T> list, MutableAction<T> action)
{
for (int i = 0; i < list.Count; i++)
{
T item = list[i];
action(ref item);
list[i] = item;
}
}

static void Main()
{
List<string> haiku = new List<string>();

haiku.Add("All software changes");
haiku.Add("Upon further reflection -");
haiku.Add("A reference to String");

MutableForEach(haiku,
delegate(ref string s)
{
s = s.ToLower();
});

haiku.ForEach(Console.WriteLine);
}
}

Of course, it's probably best to just use the List.ConvertAll<T>() method.

Solution #2:

using System;
using System.Collections.Generic;

class WWWTC
{
static void Main()
{
List<string> haiku = new List<string>();

haiku.Add("All software changes");
haiku.Add("Upon further reflection -");
haiku.Add("A reference to String");

// make it lowercase
haiku = haiku.ConvertAll<string>(
delegate(string s)
{
return s.ToLower();
});

// ... and now print it out
haiku.ForEach(Console.WriteLine);
}
}

Or, the conversion could be removed and the ToLower() call could be moved to the last ForEach.

Solution #3:

using System;
using System.Collections.Generic;

class WWWTC
{
static void Main()
{
List<string> haiku = new List<string>();

haiku.Add("All software changes");
haiku.Add("Upon further reflection -");
haiku.Add("A reference to String");

// ... and now print it out
haiku.ForEach(
delegate(string s)
{
Console.WriteLine(s.ToLower());
});
}
}

And, there's the more declarative way of doing this which tends to work better when dealing with immutable data:

Solution #4a (in C# 2.0):

using System;
using System.Collections.Generic;

class WWWTC
{
static void Main()
{
List<string> haiku = new List<string>();

haiku.Add("All software changes");
haiku.Add("Upon further reflection -");
haiku.Add("A reference to String");

haiku.ConvertAll<string>(
delegate(string s)
{
return s.ToLower();
}
).ForEach(Console.WriteLine);
}
}

Solution #4b (in C# 3.0):

using System;
using System.Collections.Generic;

class WWWTC
{
static void Main()
{
List<string> haiku = new List<string>();

haiku.Add("All software changes");
haiku.Add("Upon further reflection -");
haiku.Add("A reference to String");

haiku.ConvertAll(s => s.ToLower()).ForEach(Console.WriteLine);
}
}
Jun Meng Tuesday, April 17, 2007
In .NET, the parameter is passed by value, but what is the "value"? For reference type, the value is the "value of reference". String is a reference type, so the parameter passed in is actually a copy of the reference value of the string.

Because string is immutable, .Net creates another string in the delegate and assign the new reference to the parameter. Now s is pointing to a new string, but the original caller's string reference is still pointing to the old string.
Jun Meng Tuesday, April 17, 2007
Another similar example is like this:

public class Person
{
public int Id;
}

public void ChangeIt(Person per)
{
per = new Person();
per.Id = 2;
}

Main method:
-------------------

Person p = new Person();
p.Id = 1;

ChangeIt(p);

What will be the Id of p at this time? It will be still 1.

Same thing happens to the parameter s in the blog post.
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!