Dejan
Dejan

Reputation: 1028

Update class property inside Enumerable.Zip linq c#

I want to update every class property in IEnumerable from the external list.

class MyClass
{
    public int Value { get; set; }            
}

I tried the following

var numbers = new List<int>() { 1, 2, 3 };
var myclasses = Enumerable.Repeat(new MyClass(), numbers.Count);  

myclasses.Zip(numbers, (a, b) => a.Value = b).ToList();           // Set each property [Not working as expected]
myclasses.ToList().ForEach(x => Console.WriteLine(x.Value));      // Print all properties

I expect that previous code will set Value property in myclasses with 1,2,3 respectivelly. But this code set 3 in every property and prints:

OUTPUT:
3
3
3

I don't understand this behavior. Can someone explain me why this happened and what is proper way to do this using LINQ.

Upvotes: 1

Views: 663

Answers (1)

Ren&#233; Vogt
Ren&#233; Vogt

Reputation: 43916

Enumerable.Repeat(a, b) generates a sequence that contains b times a. So you get a sequence that contains three times the same instance (you only instantiate one new MyClass()).

So by the last execution of a.Value = b you set the Value property of your single MyClass instance to 3, and the x in your WriteLine is always the same instance, containing Value = 3.

A better way to create myclasses would be

var myclasses = Enumerable.Range(1, numbers.Count).Select(i => new MyClass());

Note that it's often a bad idea to create sequences with side effects. You actually don't use the sequence returned by Zip but use this query only for it's side effects inside your lambda.

I guess in your real application the creation of myclasses and the update via Zip is more separated than in your question. But if not, you could reduce it all to this:

var myclasses = numbers.Select(i => new MyClass { Value = i });

UPDATE

The reason why you now only get 0s as output is the deferred exeuction (again: using functional programming methods like linq only for its side effects is tricky).

The immediate solution is to add ToList to the creation:

var myclasses = Enumerable.Range(1, numbers.Count).Select(i => new MyClass()).ToList();

Without that, the classes are instantiated only at this line:

myclasses.Zip(numbers, (a, b) => a.Value = b).ToList(); 

And the created classes are returned as a sequence which is then turned in to a List by ToList(). But you don't use that return value.

So in the next line

myclasses.ToList().ForEach(x => Console.WriteLine(x.Value));

you creat new instances of the classes.

myclasses as you defined it is only a query, not the answer to that query.

So my final proposal:

var myclasses = Enumerable.Range(1, numbers.Count).Select(i => new MyClass());
var updatedClasses = myclasses.Zip(numbers, (a, b) => {a.Value = b; return a;}).ToList(); 
updatedClasses.ForEach(x => Console.WriteLine(x.Value));

Note that I changed the lambda in Zip to return the instance of the class instead of the int.

Upvotes: 2

Related Questions