Error: "Cannot modify the return value" c#

Question

I'm using auto-implemented properties. I guess the fastest way to fix following is to declare my own backing variable?

public Point Origin { get; set; }

Origin.X = 10; // fails with CS1612

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.

Solution 1

This is because Point is a value type (struct).

Because of this, when you access the Origin property you're accessing a copy of the value held by the class, not the value itself as you would with a reference type (class), so if you set the X property on it then you're setting the property on the copy and then discarding it, leaving the original value unchanged. This probably isn't what you intended, which is why the compiler is warning you about it.

If you want to change just the X value, you need to do something like this:

Origin = new Point(10, Origin.Y);

Solution 2

Using a backing variable won't help. The Point type is a Value type.

You need to assign the whole Point value to the Origin property:-

Origin = new Point(10, Origin.Y);

The problem is that when you access the Origin property what is returned by the get is a copy of the Point structure in the Origin properties auto-created field. Hence your modification of the X field this copy would not affect the underlying field. The compiler detects this and gives you an error since this operation is entirely useless.

Even if you used your own backing variable your get would look like:-

get { return myOrigin; }

You'd still be returning a copy of the Point structure and you'd get the same error.

Hmm... having read your question more carefully perhaps you actually mean to modify the backing variable directly from within your class:-

myOrigin.X = 10;

Yes that would be what you would need.

Solution 3

By now you already know what the source of the error is. In case a constructor doesn't exist with an overload to take your property (in this case X), you can use the object initializer (which will do all the magic behind the scenes). Not that you need not make your structs immutable, but just giving additional info:

struct Point
{
    public int X { get; set; }
    public int Y { get; set; }
}

class MyClass
{
    public Point Origin { get; set; }
}

MyClass c = new MyClass();
c.Origin.X = 23; //fails.

//but you could do:
c.Origin = new Point { X = 23, Y = c.Origin.Y }; //though you are invoking default constructor

//instead of
c.Origin = new Point(23, c.Origin.Y); //in case there is no constructor like this.

This is possible because behind the scenes this happens:

Point tmp = new Point();
tmp.X = 23;
tmp.Y = Origin.Y;
c.Origin = tmp;

This looks like a very odd thing to do, not at all recommended. Just listing an alternate way. The better way to do is make struct immutable and provide a proper constructor.

Solution 4

I think a lot of people are getting confused here, this particular issue is related to understanding that value type properties return a copy of the value type (as with methods and indexers), and value type fields are accessed directly. The following code does exactly what you are trying to achieve by accessing the property's backing field directly (note: expressing a property in its verbose form with a backing field is the equivalent of an auto property, but has the advantage that in our code we can access the backing field directly):

class Program
{
    static void Main(string[] args)
    {
        var myClass = new MyClass();
        myClass.SetOrigin();
        Debug.Assert(myClass.Origin.X == 10); //succeeds
    }
}

class MyClass
{
    private Point _origin;
    public Point Origin
    { 
        get => _origin; 
        set => _origin = value; 
    }

    public void SetOrigin()
    {
        _origin.X = 10; //this works
        //Origin.X = 10; // fails with CS1612;
    }
}

The error you are getting is an indirect consequence of not understanding that a property returns a copy of a value type. If you are returned a copy of a value type and you do not assign it to a local variable then any changes you make to that copy can never be read and therefore the compiler raises this as an error since this cannot be intentional. If we do assign the copy to a local variable then we can change the value of X, but it will only be changed on the local copy, which fixes the compile time error, but will not have the desired effect of modifiying the Origin property. The following code illustrates this, since the compilation error is gone, but the debug assertion will fail:

class Program
{
    static void Main(string[] args)
    {
        var myClass = new MyClass();
        myClass.SetOrigin();
        Debug.Assert(myClass.Origin.X == 10); //throws error
    }
}

class MyClass
{
    private Point _origin;
    public Point Origin
    { 
        get => _origin; 
        set => _origin = value; 
    }

    public void SetOrigin()
    {
        var origin = Origin;
        origin.X = 10; //this is only changing the value of the local copy
    }
}

Solution 5

Aside from debating the pros and cons of structs versus classes, I tend to look at the goal and approach the problem from that perspective.

That being said, if you don't need to write code behind the property get and set methods (as in your example), then would it not be easier to simply declare the Origin as a field of the class rather than a property? I should think this would allow you to accomplish your goal.

struct Point
{
    public int X { get; set; }
    public int Y { get; set; }
}

class MyClass
{
    public Point Origin;
}

MyClass c = new MyClass();
c.Origin.X = 23;   // No error.  Sets X just fine

View more solutions

Comments

[Eric Lippert]

This is yet another illustration of why mutable value types are a bad idea. If you can avoid mutating a value type, do so.

[Tom Wilson]

Take the following code (from my efforts at an AStar implementation blogged by a certain EL :-), which could not avoid changing a value type: class Path<T> : IEnumerable<T> where T : INode, new() {...} public HexNode(int x, int y) : this(new Point(x, y)) {} Path<T> path = new Path<T>(new T(x, y)); // Error // Ugly fix Path<T> path = new Path<T>(new T()); path.LastStep.Centre = new Point(x, y);

[Hans Kesting]

The important point is that Point is a struct (valuetype). If it was a class (object) then the original code would have worked.

[Meydjer Luzzoli]

If the Point is a member of a reference type then it will not be on the stack, it will be on the heap in the containing object's memory.

[supercat]

Wouldn't that trash out the value of Origin.Y? Given a property of type Point, I would think the idiomatic way to change just X would be var temp=thing.Origin; temp.X = 23; thing.Origin = temp;. The idiomatic approach has the advantage that it doesn't have to mention the members it doesn't want to modify, a feature which is only possible because Point is mutable. I'm puzzled at the philosophy that says that because the compiler can't allow Origin.X = 23; one should design a struct to require code like Origin.X = new Point(23, Origin.Y);. The latter seems really icky to me.

[nawfal]

@supercat this is the first time I'm thinking of your point, makes a lot of sense! Do you have an alternate pattern/design idea to tackle this? It would have been easier had C# not provided the default constructor for a struct by default (in that case I strictly have to pass both X and Y to specific constructor). Now it loses the point when one can do Point p = new Point(). I know why its really required for a struct, so no point in thinking abt it. But do you have a cool idea to update just one property like X?

[supercat]

For structs which encapsualate a collection of independent but related variables (such as coordinates of a point), my preference is to simply have the struct expose all its members as public fields; to modify one member of a struct property, simply read it out, modify the member, and write it back. It would have been nice if C# had provided a "simple Plain-Old-Data-Struct" declaration which would automatically define a constructor whose parameter list matched the field list, but the people responsible for C# despise mutable structs.

[nawfal]

@supercat I get it. The inconsistent behaviour of structs and classes is confusing.

[supercat]

@HansKesting: If Point were a mutable class type, the original code would have set field or property X in the object returned by property Origin. I see no reason to believe that would have the desired effect upon the object containing the Origin property. Some Framework classes have properties that copy their state to new mutable class instances and return those. Such a design has the advantage of allowing code like thing1.Origin = thing2.Origin; to set the state of object's origin to match that of another, but it can't warn about code like thing1.Origin.X += 4;.

[supercat]

The confusion results from the IMHO unhelpful belief that everything should behave like a class object. While it's useful to have a means of passing value-type values to things that expect heap object references, it's not useful to pretend value-type variables hold things which derive from Object. They don't. Every value-type definition actually defines two kinds of things: a storage location type (used for variables, array slots, etc.) and a heap object type, sometimes referred to as a "boxed" type (used when a value-type value is stored to a reference-type location).

[supercat]

If one says object o = 5;, the o will hold a reference to an instance of heap-object type System.Int32, which derives from System.Object. If, however, one instead says int i = 5;, then i will hold the bit pattern 0000...00000101. If one regards value types as existing in their own "universe" distinct from heap object types, their behavior will make sense. If one pretends that they're derivatives of System.Object, however, it won't. I suggest adopting the world-view which matches the way things actually behave.

[nawfal]

@supercat I understand the conceptual difference between the two, but again thanks for explanation. My original comment was a musing than any questioning..

[Doug]

@Paul: Do you have the ability to change the struct to a class?

[Alexander]

This is kind of a bummer, because the property setter im assigning has a side effect (the struct acts as a view into a backing reference type)

[Artorias2718]

Another solution is to simply make your struct into a class. Unlike in C++, where a class and a struct differ only by the default member access (private and public, respectively), structs and classes in C# have a few more differences. Here's some more info: docs.microsoft.com/en-us/dotnet/csharp/programming-guide/…

[rimes]

Nope, just for information. This doesn't work too

[Mitselplik]

You would be wrong. The example I posted above works just fine in a .Net Fiddle example I posted here dotnetfiddle.net/ajdvII to demonstrate.