4

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?!.

serhio
  • 28,010
  • 62
  • 221
  • 374

3 Answers3

12

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.

Community
  • 1
  • 1
Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
  • What about the error `Cannot modify the return value of 'List.this[int]' because it is not a variable ` – serhio Jul 07 '10 at 14:31
  • 1
    @serhio - Same reason. The indexer on List 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 Jul 07 '10 at 14:35
  • @serhio - If you're not using an Array, yes. – Justin Niessner Jul 07 '10 at 14:37
  • What if I have a List of Drawing.Point s? I can't redefine a Point to be a class... – serhio Jul 07 '10 at 14:46
  • @serhio - The same concept applies. Define a new Point and replace the old one. – Justin Niessner Jul 07 '10 at 14:50
  • @serhio - Ewww...I never knew that. Good find. – Justin Niessner Jul 07 '10 at 15:13
  • @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 Jul 07 '10 at 15:18
  • forgot about VB. it was a confusion... In the project I didn't use Points, it comes in mind after the answers. Thanks :) – serhio Jul 07 '10 at 15:20
2

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).

Glorfindel
  • 21,988
  • 13
  • 81
  • 109
Dan Tao
  • 125,917
  • 54
  • 300
  • 447
  • So, If my Foo should be a Point I need to do `points[0] = new Point(points[0].X, points[0].Y + 5)`?.. – serhio Jul 07 '10 at 15:05
  • 2
    @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 Jul 07 '10 at 15:12
  • I believe compiler should optimize `points[0].X, points[0].Y` calling the function only once. – serhio Jul 07 '10 at 15:22
  • @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 Jul 07 '10 at 15:25
0

'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
  • 7,251
  • 4
  • 37
  • 52
  • @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. – Dirk Vollmar Jul 07 '10 at 14:46