36
votes

I have this struct:

struct Map
{
    public int Size;

    public Map ( int size )
    {
        this.Size = size;
    }

    public override string ToString ( )
    {
        return String.Format ( "Size: {0}", this.Size );
    }
}

When using array, it works:

Map [ ] arr = new Map [ 4 ] {
    new Map(10),
    new Map(20),
    new Map(30),
    new Map(40)};

arr [ 2 ].Size = 0;

But when using List, it doesn't compile:

List<Map> list = new List<Map> ( ) {
    new Map(10),
    new Map(20),
    new Map(30),
    new Map(40)};

list [ 2 ].Size = 0;

Why?

6
What do you mean it does not work? Does it compile? Does it throw a run-time exception? Does it run but the property does not change accordingly?Andrei Rînea
Sorry haven't seen this issue ever before..Andrei Rînea
@Peter Mortensen: there is no need to add "C#" to the title. SO handles that just fine since it's a tagdefault

6 Answers

51
votes

The C# compiler will give you the following error:

Cannot modify the return value of 'System.Collections.Generic.List.this[int]' because it is not a variable

The reason is that structs are value types so when you access a list element you will in fact access an intermediate copy of the element which has been returned by the indexer of the list.

From MSDN:

Error Message

Cannot modify the return value of 'expression' because it is not a variable

An attempt was made to modify a value type that was the result of an intermediate expression. Because the value is not persisted, the value will be unchanged.

To resolve this error, store the result of the expression in an intermediate value, or use a reference type for the intermediate expression.

Solutions:

  1. Use an array. This gives you direct access to the elements (you are not accessing a copy)
  2. When you make Map a class you can still use a List to store your element. You will then get a reference to a Map object instead of an intermediate copy and you will be able to modify the object.
  3. If you cannot change Map from struct to a class you must save the list item in a temporary variable:

 

List<Map> list = new List<Map>() { 
    new Map(10), 
    new Map(20), 
    new Map(30), 
    new Map(40)
};

Map map = list[2];
map.Size = 42;
list[2] = map;
18
votes

Because it is a struct, when using the List<T>, you're are creating copies.

When using a struct, it is better to make them immutable. This will avoids effects like this.

When using an array, you have direct access to the memory structures. Using the List<T>.get_Item you work on a return value, that is, a copy of the structure.

If it was a class, you would get a copy of a pointer to the class, but you would not notice this, since pointers are hidden in C#.

Also using the List<T>.ToArray does not solve it, because it will create a copy of the internal array, and return this.

And this is not the only place where you will get effects like this when using a struct.

The solution provided by Divo is a very good workaround. But you have to remember to work this way, not only when using the List<T> but everywhere you want to change a field inside the struct.

2
votes

I'm not an XNA developer, so I cannot comment on how strict is the requirement to use structs vs. classes for XNA. But this does seem to be a much more natural place to use a class in order to get the pass-by-ref semantics you are looking for.

One thought I had is that you could make use of boxing. By creating an interface, say, 'IMap' in your case, to be implanted by your 'Map' struct, and then using a List<IMap>, the list will be holding System.Object objects, which are passed by reference. For example:

interface IMap
{
    int Size { get; set; }
}

struct Map: IMap
{
    public Map(int size)
    {
        _size = size;
    }

    private int _size;

    public int Size
    {
        get { return _size; }
        set { _size = value; }
    }

    public override string ToString()
    {
        return String.Format("Size: {0}", this.Size);
    }

}

Which could then be called by the following:

List<IMap> list = new List<IMap>() { 
    new Map(10), 
    new Map(20), 
    new Map(30), 
    new Map(40)};

    list[2].Size = 4;
    Console.WriteLine("list[2].Size = " + list[2].Size.ToString());

Note that these structs would only be boxed once, when passed into the List in the first place, and NOT when called using code such as 'list[2].Size = 4', so it should be fairly efficient, unless you are taking these IMap objects and casting back to Map (copying it out of the List<IMap>) in other parts of your code.

Although this would achieve your goal of having direct read-write access to the structs within the List<>, boxing the struct is really stuffing the struct into a class (a System.Object) and I would, therefore, think that it might make more sense to make your 'Map' a class in the first place?

Mike

2
votes

I keep coming back to this question when trying to calculate normals on a vertex buffer in XNA.

The best XNA solution I came up with was to copy the data (or store it) in an array.

private void SomeFunction()
{
    List<VertexBasicTerrain> vertexList = GenerateVertices();
    short[] indexArray = GenerateIndices();

    CalculateNormals(vertexList, ref indexArray); // Will not work


    var vertexArray = vertexList.ToArray();
    CalculateNormals(ref vertexArray, ref indexArray);
}

// This works (algorithm from Reimers.net)
private void CalculateNormals(ref VertexBasicTerrain[] vertices, ref short[] indices)
{
    for (int i = 0; i < vertices.Length; i++)
        vertices[i].Normal = new Vector3(0, 0, 0);

    for (int i = 0; i < indices.Length / 3; i++)
    {
        Vector3 firstvec = vertices[indices[i * 3 + 1]].Position - vertices[indices[i * 3]].Position;
        Vector3 secondvec = vertices[indices[i * 3]].Position - vertices[indices[i * 3 + 2]].Position;
        Vector3 normal = Vector3.Cross(firstvec, secondvec);
        normal.Normalize();
        vertices[indices[i * 3]].Normal += normal;
        vertices[indices[i * 3 + 1]].Normal += normal;
        vertices[indices[i * 3 + 2]].Normal += normal;
    }
    for (int i = 0; i < vertices.Length; i++)
        vertices[i].Normal.Normalize();
}

// This does NOT work and throws a compiler error because of the List<T>
private void CalculateNormals(List<VertexBasicTerrain> vertices, ref short[] indices)
{
    for (int i = 0; i < vertices.Length; i++)
        vertices[i].Normal = new Vector3(0, 0, 0);

    for (int i = 0; i < indices.Length / 3; i++)
    {
        Vector3 firstvec = vertices[indices[i * 3 + 1]].Position - vertices[indices[i * 3]].Position;
        Vector3 secondvec = vertices[indices[i * 3]].Position - vertices[indices[i * 3 + 2]].Position;
        Vector3 normal = Vector3.Cross(firstvec, secondvec);
        normal.Normalize();
        vertices[indices[i * 3]].Normal += normal;
        vertices[indices[i * 3 + 1]].Normal += normal;
        vertices[indices[i * 3 + 2]].Normal += normal;
    }
    for (int i = 0; i < vertices.Length; i++)
        vertices[i].Normal.Normalize();
}
1
votes

I decided to directly replace the result on a copy and reassign the result like:

Map map = arr [ 2 ];
map.Size = 0;
arr [ 2 ] = map;
1
votes
list [ 2 ].Size = 0;

is in fact:

//Copy of object
Map map = list[2];
map.Size = 2;

Use class in place of struct.