37

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?

default
  • 11,485
  • 9
  • 66
  • 102
Joan Venge
  • 315,713
  • 212
  • 479
  • 689
  • 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 Jan 06 '09 at 00:02
  • 1
    Sorry haven't seen this issue ever before.. – Andrei Rînea Jan 06 '09 at 00:18
  • @Peter Mortensen: there is no need to add "C#" to the title. SO handles that just fine since [it's a tag](http://meta.stackexchange.com/q/19190/147063) – default Sep 03 '12 at 15:01

6 Answers6

53

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;
Dirk Vollmar
  • 172,527
  • 53
  • 255
  • 316
  • Thanks but I am using XNA framework and most stuff there is implemented as a struct, like Vector3, etc. – Joan Venge Jan 06 '09 at 00:18
  • 1
    It's just a C# limitation, the CLR has the possibility to return references to structures (managed pointers), but the C# language doesn't implement that. – Pop Catalin Jan 06 '09 at 01:04
  • Thanks, but for arrays it returns references? – Joan Venge Jan 06 '09 at 01:25
  • 2
    Yes, http://msdn.microsoft.com/en-us/library/system.reflection.emit.opcodes.ldelema.aspx, but managed pointers & can be returned from methods too, C# doesn't do it. – Pop Catalin Jan 06 '09 at 02:17
19

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.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
GvS
  • 52,015
  • 16
  • 101
  • 139
  • 2
    How do you make it immutable? – Joan Venge Jan 06 '09 at 00:10
  • Don't provide any way of modifying the values inside the struct. Like .NET does with strings and DateTime objects, all operations should return a new instance, rather than changing the current instance. – Joel Mueller Jan 06 '09 at 00:11
  • But what about instance methods? – Joan Venge Jan 06 '09 at 00:13
  • Change the code for the Size field to (something like): public int Size { get; private set; } – GvS Jan 06 '09 at 00:15
  • 1
    If you need to change the Size (or other fields) a lot, better to go for a class, and not use sctruct. – GvS Jan 06 '09 at 00:17
  • Since the OP requires that this be a struct, couldn't one use 'unsafe' code to directly access the structure? – Mike Rosenblum Jan 06 '09 at 00:58
  • That's interesting. Do you think this would be the only way to get a handle of a struct? But even so why the ones in the array works as expected but not in the List? Looks like an overlook of the .NET team? – Joan Venge Jan 06 '09 at 01:24
  • Concerning your last paragraph: You actually don't have to remember to work this way. The compiler will throw an error at you when you try to assign to an intermediate result value. – Dirk Vollmar Jan 06 '09 at 12:01
2
list [ 2 ].Size = 0;

is in fact:

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

Use class in place of struct.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Michał Ziober
  • 37,175
  • 18
  • 99
  • 146
2

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

Mike Rosenblum
  • 12,027
  • 6
  • 48
  • 64
2

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();
}
BinaryConstruct
  • 191
  • 1
  • 5
1

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;
Joan Venge
  • 315,713
  • 212
  • 479
  • 689