LINQ statement no longer works after being wrapped into an extension method

Question

I had a need for a method that could take a collection of strings, and replace all occurrences of a specific string with another.

For example, if I have a List<string> that looks like this:

List<string> strings = new List<string> { "a", "b", "delete", "c", "d", "delete" };

and I want to replace "delete" with "", I would use this LINQ statement:

strings = (from s in strings select (s=="delete" ? s=String.Empty : s)).ToList();

and it works great. But then I figured I should make it an extension method, since I'd likely use it again later. In this case, I just want to write the following:

strings.ReplaceStringInListWithAnother( "delete", String.Empty);

While my code compiles, and the LINQ statement works inside of the extension method, when I return the collection reverts back to its original contents:

public static void ReplaceStringInListWithAnother( this List<string> my_list, string to_replace, string replace_with)
{
    my_list = (from s in my_list select (s==to_replace ? s=replace_with : s)).ToList();
}

So it would seem that I just modified a copy of the List... but when I looked at the code for Pop, it modifies the collection similarly, yet the changes stick, so my assumption was that my method's parameter declarations are correct.

Can anyone explain what I am doing wrong here?

Answer 1

The LINQ statement you wrote does not modify the collection, it actually creates a new one.

The extension method you wrote creates this new collection and then discards it. The assignment is redundant: you’re assigning to a local parameter, which goes out of scope immediately after.

When you’re calling the method, you’re also discarding its result instead of assigning it back.

Therefore, you should write the method like this:

public static List<string> ReplaceStringInListWithAnother(
    this List<string> my_list, string to_replace, string replace_with)
{
    return (from s in my_list select
        (s == to_replace ? replace_with : s)).ToList();
}

and the call like this:

strings = strings.ReplaceStringInListWithAnother("delete", "");

---

By the way, you can make the function more useful by making it generic:

public static List<T> ReplaceInList<T>(this List<T> my_list,
    T to_replace, T replace_with) where T : IEquatable<T>
{
    return (from s in my_list select
        (s.Equals(to_replace) ? replace_with : s)).ToList();
}

This way you can use it for other things, not just strings. Furthermore, you can also declare it to use IEnumerable<T> instead of List<T>:

public static IEnumerable<T> ReplaceItems<T>(this IEnumerable<T> my_list,
    T to_replace, T replace_with) where T : IEquatable<T>
{
    return from s in my_list select (s.Equals(to_replace) ? replace_with : s);
}

This way you can use it for any collection of equatable items, not just List<T>. Notice that List<T> implements IEnumerable<T>, so you can still pass a List into this function. If you want a list out, simply call .ToList() after the call to this one.

Update: If you actually want to replace elements in a list instead of creating a new one, you can still do that with an extension method, and it can still be generic, but you can’t use Linq and you can’t use IEnumerable<T>:

public static void ReplaceInList<T>(this List<T> my_list,
    T to_replace, T replace_with) where T : IEquatable<T>
{
    for (int i = 0; i < my_list.Count; i++)
        if (my_list[i].Equals(to_replace))
            my_list[i] = replace_with;
}

This will not return the new list, but instead modify the old one, so it has a void return type like your original.

Answer 2

You haven't shown the code for "Pop" so it's hard to know what you mean. You talk about "when I return the collection" but you're not returning anything - the method has a void return type.

LINQ typically doesn't change the contents of an existing collection. Usually you should return a new collection from the extension method. For example:

public static IEnumerable<string> ReplaceAll
    (this IEnumerable<string> myList, string toReplace, string replaceWith)
{
    return toReplace.Select(x => x == toReplace ? replaceWith : x);
}

(I've made it more general here - you shouldn't start materializing lists unless you really need to.)

You'd then call it with:

strings = strings.ReplaceAll("delete", "").ToList();

... or change the type of string to IEnumerable<string> and just use

strings = strings.ReplaceAll("delete", "");

Answer 3

Here's a hint: what do you expect the below code to do?

void SetToTen(int y)
{
    y = 10;
}

int x = 0;
SetToTen(x);

Hopefully, you understand that the SetToTen method above does nothing meaningful, since it only changes the value of its own local variable y and has no effect on the variable whose value was passed to it (in order for that to happen, the y parameter would have to be of type ref int and the method would be called as SetToTen(ref x)).

Keeping in mind that extension methods are really just static methods in fancy clothes, it should be clear why your ReplaceStringInListWithAnother is not doing what you expected: it is only setting its local my_list variable to a new value, having no effect on the original List<string> passed to the method.

Now, it's worth mentioning that the only reason this is not working for you is that your code works by setting a variable to a new object*. If you were to modify the List<string> passed to ReplaceStringInListWithAnother, everything would work just fine:

public static void ReplaceStringInListWithAnother( this List<string> my_list, string to_replace, string replace_with)
{
    for (int i = 0; i < my_list.Count; ++i)
    {
        if (my_list[i] == to_replace)
        {
            my_list[i] = replace_with;
        }
    }
}

It's also worth mentioning that List<string> is an overly restrictive parameter type for this method; you could achieve the same functionality for any type implementing IList<string> (and so I'd change the my_list parameter to be of type IList<string>).

---

*Reading your question again, it seems clear to me that this is the main point of confusion for you. The important thing you have to realize is that by default, everything in C# is passed by value. With value types (anything defined as a struct -- int, double, DateTime, and many more), the thing that's passed is the value itself. With reference types (anything that's defined as a class), the thing that's passed is a reference to an object. In the latter case, all method calls on references to objects of mutable types do actually affect the underlying object, since multiple variables of reference type can point to the same object. But assignment is different from a method call; if you assign a reference to an object that has been passed by value to some new reference to an object, you are doing nothing to the underlying object, and therefore nothing is happening that would be reflected by the original reference.

This is a really important concept that many .NET developers struggle with. But it's also a topic that's been explained thoroughly elsewhere. If you need more explanation, let me know and I'll try to dig up a link to a page that makes all of this as clear as possible.