Why my method changes the original value of the object that is passed to it?

Question

I have the following object

var filters = new List<IReportFilter>
{
    new ReportFilter
    { 
        ReportColumn = new ReportColumn{ ColumnKey = "Result.IsCompleted"},
        Value = "1",
        SubFilters = new List<IReportFilter> 
        {
             new ReportFilter { SqlOperator = FilterOperator.Or, ReportColumn = new ReportColumn{ ColumnKey = "User.LastName"}, Value = "Alhayek"},
             new ReportFilter { SqlOperator = FilterOperator.Or, ReportColumn = new ReportColumn{ ColumnKey = "User.LastName"}, Value = "Smith"},
             new ReportFilter { SqlOperator = FilterOperator.Or, ReportColumn = new ReportColumn{ AggregateFunction = SqlAggregateFunctions.Count}, Type = FilterType.GreaterThenOrEqualTo ,Value = "0" },
        }

    },

};

The obove object is passed to another class using a method like so

IReportModel ReportModel = Template.CreateReport();

ReportModel.Get(filters);

Inside the the Get method of the ReportModel class I want to loop through the filters list and create a new list without changing the original list. the new list will become a subset of the original.

From with in my Get method here is what I have done

public SqlCommand Build(List<IReportFilter> filters)
{

    var a = CloneFilters(filters);

    var b = CloneFilters(filters);

    List<IReportFilter> standardFilters = ExtractFiltersByAType(a, true);

    List<IReportFilter> aggregateFilter = ExtractFiltersByAType(b, false);

}

But every time I execute the method ExtractFiltersByAType the value of a,b, and filters change to equal the same value of aggregateFilter.

I am NOT expecting for any of the variables to change. But they are for some reason that I don't understand.

Here is my CloneFilters method

private List<IReportFilter> CloneFilters(List<IReportFilter> myFilters)
{
    List<IReportFilter> copyOfFilters = new List<IReportFilter>();

    foreach (var myFilter in myFilters)
    {
        copyOfFilters.Add(myFilter);
    }

    return copyOfFilters;
}

And here is my ExtractFiltersByAType

private List<IReportFilter> ExtractFiltersByAType(List<IReportFilter> filtersSource, bool IsStandard = true)
{
    List<IReportFilter> validFilters = new List<IReportFilter>();

    foreach (var filterSource in filtersSource)
    {

        if (filterSource.SubFilters != null && filterSource.SubFilters.Any())
        {
            filterSource.SubFilters = ExtractFiltersByAType(filterSource.SubFilters, IsStandard); //I think this what could be causing this problem
        }

        if ((IsStandard && !filterSource.ReportColumn.IsAggregate) || (!IsStandard && filterSource.ReportColumn.IsAggregate))
        {
            validFilters.Add(filterSource);
        }

    }

    return validFilters;
}

Question

Since I am not using ref to pass the object by reference to the method, why is my function changing the value to original object?

When passing a list of object to method in c#, will the system create a copy or will it passes the object by reference?

How can I solve this problem so that every time I execute ExtractFiltersByAType method, only the copy is changed not the originals?

I am thinking that the line filterSource.SubFilters = ExtractFiltersByAType(filterSource.SubFilters, IsStandard); in the ExtractFiltersByAType is causing the problem but I don't understand why and how.

your clonefilters method is shallow-copying the list; thus you are just creating a new list with the same elements. you should do a 'deep copy'. — muratgu, Jul 17 '16 at 23:43
Possible duplicate of [Are arrays or lists passed by default by reference in c#?](http://stackoverflow.com/questions/967402/are-arrays-or-lists-passed-by-default-by-reference-in-c) — Win, Jul 17 '16 at 23:44
`copyOfFilters.Add(myFilter);` is not a deep copy, its a shallow copy. — , Jul 17 '16 at 23:52

score 6 · Accepted Answer · edited May 23 '17 at 12:08

Without ref

When you pass a reference type as an argument (which includes a list), you pass a copy of the reference to that object. This means you can change your object attributes, but can't change the object itself.

Example:

public class Program
{
    static void Main(string[] args)
    {
        Foo foo = new Foo(1);
        Console.WriteLine(foo.Bar);

        // This will change foo.Bar
        ChangeFoo(foo, 5); 
        Console.WriteLine(foo.Bar);

        // Does not change foo
        DoesNotChangeFoo(foo, 10);
        Console.WriteLine(foo.Bar);

        Console.Read();
    }

    static void ChangeFoo(Foo foo, int newValue)
    {
        // Since it receives a copy of the reference to Foo, it actually changes foo.Bar value
        foo.Bar = newValue;
    }

    static void DoesNotChangeFoo(Foo foo, int newValue)
    {
        // Since it receives a copy of the reference to foo, it only updates this method's reference, not changing the caller's reference
        foo = new Foo(newValue);
    }
}

public class Foo
{
    public Foo(int bar)
    {
        Bar = bar;
    }

    public int Bar { get; set; }
}

With ref

If you wanted to change the caller's object reference, you would need to pass the actual reference used by the calle's, that's when you use the ref keyword.

Example:

public class Program
{
    static void Main(string[] args)
    {
        Foo foo = new Foo(1);
        Console.WriteLine(foo.Bar);

        // This will change foo's object reference
        ChangeFooObjectReference(ref foo, 15); 
        Console.WriteLine(foo.Bar);

        Console.Read();
    }

    static void ChangeFooObjectReference(ref Foo foo, int newValue)
    {
        // SInce you are receiving the actual reference used by the caller, you actually change it's own reference
        foo = new Foo(newValue);
    }
}

public class Foo
{
    public Foo(int bar)
    {
        Bar = bar;
    }

    public int Bar { get; set; }
}

Your case

As you correcly assumed, the main cause of your problem is this line:

filterSource.SubFilters = ExtractFiltersByAType(filterSource.SubFilters, IsStandard);

This line actually changes this object's SubFilters.

But it's worth noting that you may have some bigger problems in you Clone method.

private List<IReportFilter> CloneFilters(List<IReportFilter> myFilters)
{
    List<IReportFilter> copyOfFilters = new List<IReportFilter>();

    foreach (var myFilter in myFilters)
    {
        copyOfFilters.Add(myFilter);
    }

    return copyOfFilters;
}

This method return's a new List, but the content of that list is exactly the same as the argument's. This means that, if you change any of the object's contained in the object used as an argument, you change it in the new List too.

Here's an example of what's happening.

static void Main(string[] args)
    {
        List<Foo> foos = new List<Foo>();
        foos.Add(new Foo(2));

        List<Foo> newFoo = CreateNewFoo(foos);
        Console.WriteLine(newFoo.First().Bar);

        foos.First().Bar = 5;
        // Since we changed the first object of the old list, and it is the same object in the new list, we will get the new result.
        Console.WriteLine(newFoo.First().Bar);

        Console.Read();

    }

    static List<Foo> CreateNewFoo(List<Foo> foos)
    {
        List<Foo> newFoos = new List<Foo>();

        foreach(Foo foo in foos)
        {
            newFoos.Add(foo);
        }

        return newFoos;
    }

I would suggest implementing the ICloneable interface in your IReportFilter interface, and each concrete class implementing IReportFilter. ICloneable implements a single method Clone(), which returns an object. This method should create a new instance of the same class in which it's implemented, containing a new object identical to the current object. Than you would change your method to:

private List<IReportFilter> CloneFilters(List<IReportFilter> myFilters)
{
    List<IReportFilter> copyOfFilters = new List<IReportFilter>();

    foreach (var myFilter in myFilters)
    {
        copyOfFilters.Add(myFilter.Clone() as IReportFilter);
    }

    return copyOfFilters;
}

As for implementing the ICloneable inteface, refer to this question: Proper way to implement ICloneable

Edit

As mentioned by user muratgu in the question comments, your CloneFilter method is doing a shallow copy of your list, what you are looking for is a deep copy. That could be implemented with the aforementioned ICloneable interface.

Thank you very much! that makes since! – Jaylen Jul 18 '16 at 19:04 — Jaylen, Jul 18 '16 at 19:04

score 0 · Answer 2 · answered Jul 18 '16 at 01:53

The only thing that ref does is determine whether the method receiving the parameter can modify the variable passed to it. In the case of an object, that means setting the variable to a different object or setting it to null.

But even if you don't use ref, the method you pass the parameter to can can set its properties or call methods which modify the state of that object. That's normal and expected. When you pass an object to another method, it's not just so that the other method can read its properties. You might also want that method to operate on that object in some way that modifies it.

The simplest example is a List<T>. If you pass a List<string> to another method - without using ref - that other method can add items to the list, modify items in the list, clear the list, etc.

The only difference with using ref is that if the method you pass the variable to sets the variable to a different list or sets it to null, it's going to modify the variable in the method that passed the argument.

Most of the time we don't want a method we call to completely replace the variable we pass in. If we wanted a new object we'd write a function that returns an object, not one that replaces the variable we passed in.

Why my method changes the original value of the object that is passed to it?

2 Answers2

Without ref

With ref

Your case

Edit