36

in a file I defined a public struct

public struct mystruct
{
    public Double struct1;
    public Decimal struct2;
}

In another I tried to do this:

class Test
{
    mystruct my_va;

    public mystruct my_va
    {
        get { return my_va; }
        set { my_va = value; }
    }

    public Test()
    {
        my_va.struct1 = 10;
    }
}

Intellisense recognizes My_va.struct1 but compiler says

Error 1 Cannot modify the return value of 'TEST.mystruct' because it is not a variable

How to correct the syntax ?

Improve this question

6 Answers 6

Reset to default
56

Yup, it's absolutely right. You see, when you fetch My_va, you're fetching a value - a copy of the current value of my_va. Changing that value would have no benefit, because you'd be immediately discarding the copy. The compiler is stopping you from writing code which doesn't do what it looks like it does.

In general, avoid mutable structs. They're evil. In this case, you could (for example) change mystruct to be immutable, but with a method like this:

public mystruct WithStruct1(double newValue)
{
    return new mystruct(newValue, struct2);
}

then change your constructor code to:

My_va = My_va.WithStruct1(10);

... although in this case it's far more likely (given that you're in a constructor) that you should be writing:

My_va = new mystruct(10, 0);

Not only should structs be immutable, they should be pretty rare in most codebases, IMO. Other than for Noda Time, I've hardly ever written my own custom values types.

Finally, please learn the .NET naming conventions and try to follow them, even for sample code :)

Improve this answer
Sign up to request clarification or add additional context in comments.

14 Comments

I once heard tale of a mutable struct burning an entire village. Twas madness.
@Anthony: You "once heard a tale"? You make it sound like you don't believe it. I was there, and saw it all. It still haunts me at night.
Anakin Skywalker was cool until he learned about mutable structs.
I once defeated a mutable struct using nothing but a rake and a large glass of water.
@Marc - that depends. Does grue have a rake and a large glass of water?
|
40

It is highly recommended to avoid mutable structs. They exhibit all sorts of surprising behaviour.

Solution: Make your struct immutable.

public struct MyStruct
{
    public readonly double Value1;
    public readonly decimal Value2;

    public MyStruct(double value1, decimal value2)
    {
        this.Value1 = value1;
        this.Value2 = value2;
    }
}

Usage:

class Test
{
    private MyStruct myStruct;

    public Test()
    {
        myStruct = new MyStruct(10, 42);
    }

    public MyStruct MyStruct
    {
        get { return myStruct; }
        set { myStruct = value; }
    }
}
Improve this answer

7 Comments

@Caspar Kleijne: Value1 and Value2 are fields, not auto-properties. So, no.
Thanks that works but can you explain WHY your syntax works and not mine, I can't really understand the fundamental difference: why by making it readonly has anything to do with setting the property value ?
@user310291: As @Jon Skeet explained, structs have value semantics and you were actually modifying a field of a copy, not the struct value you intended to modify. This is a common source of errors, so even the C# compiler warns you about this. If you make a struct immutable (= all fields readonly), you can never accidentally modify a copy. (BTW the set accessor has nothing to do with this; your original code never used it.)
Well if it was only a warning it would be ok, but it just can't let me do it :)
Here's a simple analogy - int is also a value type. You can create an instance of the int value type like this - int i = 1;. You can change the value of i to 2 (i = 2) but you would never want to change the value of 1 to 2.
|
3

I work with a list of structs, and solved this in a different way.

struct Pixel
{ Public int X;
  Public int C;
}
List<Pixel> PixelList = new List<Pixel>
TempPixel = new Pixel();

Now when i want to set a value i code like this :

TempPixel = PixelList[i];
TempPixel.X= 23;  // set some value
PixelList[i] = TempPixel

The code looks a bit strange perhaps, but it resolves the issue . It solves the problem that a struct cannot be directly assigned a single value, but can be a copy of a similar type. solving error CS1612:

https://msdn.microsoft.com/query/dev10.query?appId=Dev10IDEF1&l=EN-US&k=k%28CS1612%29;k%28TargetFrameworkMoniker-%22.NETFRAMEWORK%2cVERSION%3dV2.0%22%29;k%28DevLang-CSHARP%29&rd=true

Improve this answer

4 Comments

The code doesn't look at all strange. Rather, it's the right approach if one is unable or unwilling to use an array of structures. Some people dislike mutable structs, because they don't behave like objects, but in cases where one wants a bunch of variables stuck together with duct tape I'd say it's better to use a bunch of variables stuck together with duct tape than try to make a struct behave as a mediocre imitation of an object which is doing a poor imitation of a bunch of variables stuck together with duct tape. The one thing I'd note...
...is that it may be more convenient and efficient to replace List<Pixel> PixelList with Pixel[] Pixels = new Pixel[16]; /* Or some reasonable default */ int PixelCount;, and add void AddPixel(Pixel newPixel) { if (PixelCount >= Pixels.Count) Pixels = Array.Resize(ref Pixels, PixelCount*2); Pixels[PixelCount] = newPixel; PixelCount++; }. Doing that will allow array elements to be updated in-place.
your right, i use it in some 'dangerous' multi-threading code, as a global variable-list. i get out of the danger zone, because my code runs really fast using structs. I dont think structs should be used a lot too, but in weird cases i do strange stuff with pointcloud data on dedicated hardware, then it might be better; before using a struct i recommend people to look if they realy need it. Speed and low Memory consumptions, might be reason.
IMHO, open-field structures are semantically the proper thing to use if each item in a list will encapsulate some values which will often be worked with as a unit, but should not have any identity as a unit. A Pixel[100] will encapsulate 100 independent Decimal values and 100 independent double values. If Pixel were a mutable class, a Pixel[100] might encapsulate 100+100 independent values, but it wouldn't have to. It could hold 100 references to the same instance. That adds a lot of extra potential state which is often not semantically desirable.
2

Simplest fix: Change the struct to a class.

Improve this answer

Comments

1

Unfortunately this error can be incorrectly generated when assigning to a property (i.e. invoking a property setter). An immutable struct can still have a valid property setter, as long as the property setter doesn't actually assign to any fields in the struct. For example, 

public struct Relay
{
    public Relay(Func<string> getText, Action<string> setText)
    {
        this.GetText = getText;
        this.SetText = setText;
    }
    private readonly Func<string> GetText;
    private readonly Action<string> SetText;

    public string Text {
        get { return this.GetText(); }
        set { this.SetText(value); }
    }
}

class Example
{
    private Relay Relay {
        get { return new Relay(() => this.text, t => { this.text = t; }); }
    }

    private string text;


    public Method()
    {
        var r = new Relay();
        r.Text = "hello"; // not a compile error (although there is a null reference)

        // Inappropriately generates a compiler error
        this.Relay.Text = "hello";

        r = this.Relay;
        r.Text = "hello"; // OK
    }
}
Improve this answer

1 Comment

The "error" CS 1612 should be a warning. How can I feed this back to Microsoft so that it gets it fixed? I don't want to use a class because, obviously, that involves overhead that is not appropriate in my case.
0

Worth noting, that you can overcome this behavior by:

  1. Implementing interface by struct: Struct : IStruct
  2. Declare struct as field: Struct strExplicitly;

Example:

public interface IStruct
{
    int Age { get; set; }
}

public struct Struct : IStruct
{
    public int Age { get; set; }
}

public class Test
{
    IStruct strInterface { get; set; }
    Struct strExplicitly;

    public Test()
    {
        strInterface = new Struct();
        strExplicitly = new Struct();
    }

    public void ChangeAge()
    {
        strInterface.Age = 2;
        strExplicitly.Age = 2;
    }
}
Improve this answer

1 Comment

This passes the compile but notice that interface can introduce boxing and it's better to publish public data via properties.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.