1

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.

Dejan
  • 966
  • 1
  • 8
  • 26

1 Answers1

2

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.

Community
  • 1
  • 1
René Vogt
  • 43,056
  • 14
  • 77
  • 99
  • Yes, creation of `myclasses` is separated from the question, I am only interesing in updates with `Zip`. I corrected creation of `myclassess` using `Enumerable.Range` as you wrote, but this still don't work. In that case `Zip` will do nothing (it will print all zeros). Again, I don't understand why the property setter is not called. – Dejan Mar 04 '16 at 12:51
  • I can't beleve that I made several mistakes just in few lines :). Tnx for good explanation. – Dejan Mar 04 '16 at 13:39