P a u l
P a u l

Reputation: 7911

Error: "Cannot modify the return value" c#

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.

Upvotes: 192

Views: 135312

Answers (8)

Matt
Matt

Reputation: 1544

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
    }
}

Upvotes: 7

Roberto Mutti
Roberto Mutti

Reputation: 95

Just remove the property "get set" as follow, and then everything works as always.

In case of primitive types instread use the get;set;...

using Microsoft.Xna.Framework;
using System;

namespace DL
{
    [Serializable()]
    public class CameraProperty
    {
        #region [READONLY PROPERTIES]
        public static readonly string CameraPropertyVersion = "v1.00";
        #endregion [READONLY PROPERTIES]


        /// <summary>
        /// CONSTRUCTOR
        /// </summary>
        public CameraProperty() {
            // INIT
            Scrolling               = 0f;
            CameraPos               = new Vector2(0f, 0f);
        }
        #region [PROPERTIES]   

        /// <summary>
        /// Scrolling
        /// </summary>
        public float Scrolling { get; set; }

        /// <summary>
        /// Position of the camera
        /// </summary>
        public Vector2 CameraPos;
        // instead of: public Vector2 CameraPos { get; set; }

        #endregion [PROPERTIES]

    }
}      

Upvotes: -1

Mitselplik
Mitselplik

Reputation: 1133

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

Upvotes: 2

nawfal
nawfal

Reputation: 73223

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.

Upvotes: 7

AnthonyWJones
AnthonyWJones

Reputation: 189467

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.

Upvotes: 11

MSIL
MSIL

Reputation: 2761

I guess the catch here is that you are trying to assign object's sub-values in the statement rather than assigning the object itself. You need to assign the entire Point object in this case as the property type is Point.

Point newOrigin = new Point(10, 10);
Origin = newOrigin;

Hope I made sense there

Upvotes: 0

Fredrik Norm&#233;n
Fredrik Norm&#233;n

Reputation: 19

The problem is that you point to a value located on the stack and the value will not be relfected back to the orignal property so C# does not allow you to return a reference to a value type. I think you can solve this by removing the Origin property and instead use a public filed, yes I know it's not a nice solution. The other solution is to not use the Point, and instead create your own Point type as an object.

Upvotes: 0

Greg Beech
Greg Beech

Reputation: 136667

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

Upvotes: 245

Related Questions