9

Imagine this struct :

        struct Person
        {
             public string FirstName { get; set; }
             public string LastName { get; set; }
        }

And following code :

        var list = new List<Person>();
        list.Add(new Person { FirstName = "F1", LastName = "L1" });
        list.Add(new Person { FirstName = "F2", LastName = "L2" });
        list.Add(new Person { FirstName = "F3", LastName = "L3" });

        // Can't modify the expression because it's not a variable
        list[1].FirstName = "F22";

When I want to change Property's value it gives me the following error:

Can't modify the expression because it's not a variable

While, when I tried to change it inside an array such as Person[] it worked without any error.Is there any problem with my code when using with generic collections?

Saber Amani
  • 6,409
  • 12
  • 53
  • 88
  • Why is your person no class? – Tim Schmelter Feb 18 '13 at 20:56
  • 1
    Having a mutable struct is not a good idea - can't you use a class? – Jay Feb 18 '13 at 20:57
  • It can be, but I'm curious about the reason of this error. – Saber Amani Feb 18 '13 at 20:57
  • Have you tried it without the { get; set; }, because its already public? – DROP TABLE users Feb 18 '13 at 20:59
  • 1
    Try looking at this - it's an explanation on MSDN: http://msdn.microsoft.com/query/dev10.query?appId=Dev10IDEF1&l=EN-US&k=k(CS1612);k(VS.ERRORLIST);k(TargetFrameworkMoniker-%22.NETFRAMEWORK%2cVERSION%3dV4.0%22)&rd=true - It is because value types are copied on assignment... When you retrieve a value type from a property or indexer, you are getting a copy of the object, not a reference to the object itself. – Jay Feb 18 '13 at 21:00
  • 1
    There is already a good answer to exactly the same question http://stackoverflow.com/questions/414981/directly-modifying-listt-elements – Ilya Ivanov Feb 18 '13 at 21:01
  • Possible Duplicate: http://stackoverflow.com/questions/1747654/cannot-modify-the-return-value-error-c-sharp – msmucker0527 Feb 18 '13 at 21:01
  • General rule of thumb is to never use `struct`. When you genuinely need to use a `struct`, you'll know it. Make `Person` a `class` instead. – JosephHirn Feb 18 '13 at 21:02
  • 1
    @Ginosaji how do you know it, if `General rule of thumb is to never use struct` ? – Ilya Ivanov Feb 18 '13 at 21:03
  • @Ilya Structs are generally allocated on the stack and class objects are generally allocated on the heap, so the decision is often performance related. One reason to prefer a struct over a class is when many, many instances are created and destroyed during an operation. (3D graphics rendering comes to mind) – Osiris Feb 18 '13 at 22:15
  • @IlyaIvanov: You may consider a `struct` for types that are very small, short-lived, and immutable for performance reasons ([source](http://msdn.microsoft.com/en-us/library/ms229017.aspx)), or if you simply have a need for value semantics. You probably won't see this situation come up too often, if at all. Always use `class` unless you have a specific reason to be using a `struct`. – JosephHirn Feb 19 '13 at 13:28

3 Answers3

15

When you return the struct via the List[] indexer, it returns a copy of the entry. So if you assigned the FirstName there, it would just be thrown away. Hence the compiler error.

Either rewrite your Person to be a reference type class, or do a full reassignment:

Person person = list[1];
person.FirstName = "F22";
list[1] = person;

Generally speaking, mutable structs bring about issues such as these that can cause headaches down the road. Unless you have a really good reason to be using them, you should strongly consider changing your Person type.

Why are mutable structs “evil”?

Community
  • 1
  • 1
Chris Sinclair
  • 22,858
  • 3
  • 52
  • 93
  • 2
    +1 The read/modify/writeback approach you show about is IMHO the cleanest way to code this, since it makes it very obvious what part of the structure is changed. Code like `list[1] = new Person("F22", list[1].LastName);` is far less clear, and more likely to be wrong [e.g. if `Person` has an optional `MiddleName` parameter, the latter code would accidentally clear it--oops]. – supercat Feb 18 '13 at 21:43
5

Obviously a part of the question is still unanswered. What is difference between List<Person> and Person[]. In term of getting element by index the List calls indexer (method) which returns copy of value-type instance, in opposite array by index returns not a copy but managed pointer to element at the index (used special IL instruction ldelema).

Of course mutable value-types are evil as mentioned in other answers. Look at the simple example.

var en = new {Ints = new List<int>{1,2,3}.GetEnumerator()};
while(en.Ints.MoveNext())
{
    Console.WriteLine(x.Ints.Current);
}

Surprised?

Hamlet Hakobyan
  • 32,965
  • 6
  • 52
  • 68
0

Redo your struct as such:

    struct Person
    {
         private readonly string firstName;
         private readonly string lastName;
         public Person(string firstName, string lastName)
         {
             this.firstName = firstName;
             this.lastName = lastName;
         }
         public string FirstName { get { return this.firstName; } }
         public string LastName { get { return this.lastName; } }
    }

And following code as :

    var list = new List<Person>();
    list.Add(new Person("F1", "L1"));
    list.Add(new Person("F2", "L2"));
    list.Add(new Person("F3", "L3"));

    // Can modify the expression because it's a new instance
    list[1] = new Person("F22", list[1].LastName);

This is due to the copy semantics of struct. Make it immutable and work within those constraints and the problem goes away.

Jesse C. Slicer
  • 19,901
  • 3
  • 68
  • 87
  • 1
    I dislike the "immutable" approach since one has to examine the code of the structure to know what the effect of `list[1] = new Person("F22", list[1].LastName);` is. If, for example, the `Person` class included a `MiddleName` field which would be initialized by an optional `MiddleName` parameter, the aforementioned statement would (likely accidentally) erase `list[1].MiddleName`. By contrast, if `LastName` is an exposed *field*, knowing that would be sufficient to determine that code in Chris Sinclair's answer wouldn't affect anything else. – supercat Feb 19 '13 at 15:49