Are methods that modify reference type parameters bad?

Question

I've seen methods like this:

public void Foo(List<string> list)
{
    list.Add("Bar");
}

Is this good practice to modify parameters in a method?

Wouldn't this be better?

public List<string> Foo(List<string> list)
{
    // Edit
    List<string> newlist = new List<string>(list);
    newlist.Add("Bar");
    return newlist;
}

It just feels like the first example has unexpected side effects.

Answer 1

In the example you've given, the first seems a lot nicer to me than the second. If I saw a method that accepted a list and also returned a list, my first assumption would be that it was returning a new list and not touching the one it was given. The second method, therefore, is the one with unexpected side effects.

As long as your methods are named appropriately there's little danger in modifying the parameter. Consider this:

public void Fill<T>(IList<T> list)
{
    // add a bunch of items to list
}

With a name like "Fill" you can be pretty certain that the method will modify the list.