tags:

views:

106

answers:

3

Why I am not able to modify List items?

struct Foo 
{
    public string Name;
}

Foo foo = new Foo();
foo.Name = "fooNameOne";

List<Foo> foos = new List<Foo>();
foos.Add(foo);

// Cannot modify the return value of 
// 'List<Foo>.this[int]' because it is not a variable   
//foos[0].Name = "fooNameTwo";

Foo tempFoo = foos[0];
tempFoo.Name = "fooNameTwo";

Console.WriteLine(foos[0].Name); // fooNameOne

EDIT:
I want to leave the structure for Foo. How should I proceed? foos[0] = tempFoo? little bit complicated just for an assignment?!.

+12  A: 

Because Foo is a struct and not an object. Therefore, it is a value type rather than a reference type.

When you add a value type to a List, a copy is made (unlike a reference type where the entry in the list is a reference to the original object).

The same goes for pulling an instance out of the List. When you do Foo tempFoo = foos[0], you're actually making another copy of the element at foos[0] so you're modifying the copy rather than the one in the List.

foos[0].Name = "fooNameTwo";

Is giving you an error for the same reason. The indexer of a List is still a function that returns a value. Since you're working with a value type, that function is returning a copy and storing it on the stack for you to work with. As soon as you try to modify that copy the compiler sees that you're doing something that will produce unexpected results (your changes wouldn't be reflected in element in the List) and barfs.

As Jon Skeet commented...just another reason Mutable Structs are evil (if you want more details, check out this SO question: Why are mutable structs evil?).

If you make Foo a class instead of a struct, you'll get the behavior you're looking for.

Justin Niessner
(Make Foo a class and it should work ok)
gt
What about the error `Cannot modify the return value of 'List<Foo>.this[int]' because it is not a variable `
serhio
@serhio - Same reason. The indexer on List<T> is really a function. That function returns a copy of the (because it's not a reference type) value and stores it on the stack. When you try to modify it, you're still modifying a copy...and the compiler barfs. Check here for more details: http://generally.wordpress.com/2007/06/21/c-list-of-struct/
Justin Niessner
"ou're still modifying a copy" anyway I am forced to modify a copy...
serhio
@serhio - If you're not using an Array, yes.
Justin Niessner
What if I have a List of Drawing.Point s? I can't redefine a Point to be a class...
serhio
@serhio - The same concept applies. Define a new Point and replace the old one.
Justin Niessner
@serhio - Ewww...I never knew that. Good find.
Justin Niessner
@serhio - If you really wanted to know about Points, you should've used Points in your example. Somebody would've given you the answer you got in Dan Tao's comments much sooner (and gotten the same underlying explanation from me). Just so you know for next time...
Justin Niessner
forgot about VB. it was a confusion... In the project I didn't use Points, it comes in mind after the answers. Thanks :)
serhio
A: 

'cause Foo is value type

instead of this

Foo tempFoo = foos[0];
tempFoo.Name = "fooNameTwo";

do this:

Foo tempFoo;
tempFoo = "fooNameTwo";
foos[0] = tempFoo;
Arseny
oh la... just for setting the name 3 lines of code? :(
serhio
@serhio: Use a `class` instead of a `struct`. A `struct` in .NET is almost never what you really want and it is different from C/C++ structs.
0xA3
@0xA3 you didn't read the Edit...
serhio
+1  A: 

Here's the thing.

You say this:

I want to leave the structure for Foo. How should I proceed? foos[0] = tempFoo? little bit complicated just for an assignment?!.

Yeah, well, that's just it. What you need is an assignment, not a modification. Value types (structs) are generally supposed to be treated as values.

Take int. If you had a List<int> called ints, how would you change the value at ints[0]?

Something like this, right?

ints[0] = 5; // assignment

Notice there's no way to do something like this:

ints[0].ChangeTo(5); // modification?

That's because Int32 is an immutable struct. It is intended to be treated as a value, which cannot be changed (so an int variable can only be assigned to something new).

Your Foo struct is a confusing case because it can be mutated. But since it's a value type, only copies are passed around (same as with int, double, DateTime, etc. -- all the value types we deal with on a regular basis). For this reason you cannot mutate an instance "from afar"--that is, unless it is passed to you by ref (using the ref keyword in a method call).

So the simple answer is, yeah, to change a Foo in a List<Foo>, you need to assign it to something new. But you really shouldn't have a mutable struct to begin with.

Disclaimer: As with almost any advice you can receive, in anything, this is not a 100% hard rule. The very skilled developer Rico Mariani wrote the Point3d struct to be mutable for good reasons, which he explained on his blog. But this is a case of a very knowledgeable developer knowing exactly what he's doing; in general, as a standard approach to writing value types versus reference types, value types should be made immutable.


In response to your comment: so when you are dealing with a mutable struct like Point, basically you need to do something like this:

Point p = points[0];
p.Offset(0, 5);
points[0] = p;

Or, alternatively:

Point p = points[0];
points[0] = new Point(p.X, p.Y + 5);

The reason I wouldn't do...

points[0] = new Point(points[0].X, points[0].Y + 5);

...is that here you're copying the value at points[0] twice. Remember that accessing the this property by index is basically a method call. So that code is really doing this:

points.set_Item(0, new Point(points.get_Item(0).X, points.get_Item(0).Y + 5);

Note the excessive call to get_Item (making an additional copy for no good reason).

Dan Tao
So, If my Foo should be a Point I need to do `points[0] = new Point(points[0].X, points[0].Y + 5)`?..
serhio
@serhio: I actually wouldn't do that, since every time you call `points[0]` you're getting a new copy. I'd do: `Point p = points[0]; p.Offset(0, 5); points[0] = p;`
Dan Tao
I believe compiler should optimize `points[0].X, points[0].Y` calling the function only once.
serhio
@serhio: Should it? That seems highly dubious. What if I wrote the indexer for my collection class to have side effects? (That would be almost certainly a horrible decision on my part, but I could do it.) Again, since those property accessors are basically method calls, they should **not** be "optimized" in the way you describe. Unless I'm missing something?
Dan Tao