1

Assume I have a struct Point with public int's x and y. I want to change a value and I can. But once I store it in a dictionary, I can no longer do so. Why?

MWE:

using System;
using System.Collections.Generic;

public class Program
{
    public struct Point
    {
        public int x, y;

        public Point(int p1, int p2)
        {
            x = p1;
            y = p2;
        }
    }

    public static void PrintPoint(Point p)
    {
        Console.WriteLine(String.Format("{{x: {0}, y: {1}}}", p.x, p.y));
    }

    public static void Main()
    {
        var c = new Point(5, 6);
        PrintPoint(c);                                          // {x: 5, y: 6}

        c.x = 4;  // this works
        PrintPoint(c);                                          // {x: 4, y: 6}

        var d = new Dictionary<string, Point>() 
        {
            { "a", new Point(10, 20) },
            { "b", new Point(30, 40) }
        };

        foreach (Point p in d.Values)
        {
            PrintPoint(p);  // this works
        }

        PrintPoint(d["a"]); // this works                       // {x: 10, y: 20}

        Console.WriteLine(d["a"].x.ToString());  // this works  // 10

        // d["a"].x = 2; // why doesn't this work?
    }
}

How come I can access the struct variables when it's in a dictionary but can no longer change them? How do I change them?

Arthur Dent
  • 1,828
  • 13
  • 21
  • 3
    Please see: https://stackoverflow.com/questions/441309/why-are-mutable-structs-evil –  May 18 '18 at 20:23
  • 3
    Because `d["a"]` returns a temporary copy that you aren't storing anywhere. Modifying a temporary wouldn't make sense – UnholySheep May 18 '18 at 20:23
  • It "does not work" because the compiler complains. The text of that complaint is relevant. – Hans Kesting May 18 '18 at 20:49
  • 1
    One way to solve this is to assign a new `Point` with the new `x` value and the same `y` value to the dictionary item: `d["a"] = new Point(2, d["a"].y);` – Rufus L May 18 '18 at 21:01
  • One comment from a design point of view: If you're going to have a constructor that takes values that are used to set properties (or fields in this case), it's helpful to the clients of your class/struct to make the constructor argument names the same as the property names so they understand what they're setting. For example: `public Point(int x, int y) { ... }` – Rufus L May 18 '18 at 21:03
  • Thanks, everyone! I agreed to close this as a duplicate. Ultimately, I just changed `struct` to `class` and now it's a reference type that's stored on the heap instead of a value type stored on the stack: problem solved. Thanks again! – Arthur Dent May 18 '18 at 21:03
  • @RufusL thanks, Rufus - I agree re:design. This actually looks nothing like my real struct, it's just a MWE. – Arthur Dent May 18 '18 at 21:04
  • Ah, cool. Because i was also going to comment that there's already a pretty robust [`Point Structure`](https://msdn.microsoft.com/en-us/library/system.drawing.point(v=vs.110).aspx) available in the `System.Drawing` namespace (source code for it is [here](https://referencesource.microsoft.com/#System.Drawing/commonui/System/Drawing/Point.cs,a041be61667d4c9a)) – Rufus L May 18 '18 at 21:07
  • @RufusL I did not know that! Thanks for sharing! My actual struct (now a class) stores data on installers for a program that installs a bunch of software. – Arthur Dent May 18 '18 at 21:35

2 Answers2

2

There are a couple things wrong with what you're trying to do.

In 99.999999999% of cases, structs should be readonly. See Microsoft design guidelines https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/struct

X DO NOT define mutable value types.

Mutable value types have several problems. For example, when a property getter returns a value type, the caller receives a copy. Because the copy is created implicitly, developers might not be aware that they are mutating the copy, and not the original value. Also, some languages (dynamic languages, in particular) have problems using mutable value types because even local variables, when dereferenced, cause a copy to be made.

You should also not mutate an object that is being used as a dictionary key. Dictionaries are based on hash values, and if you change a field of the key, you will change its hash value. See: https://stackoverflow.com/a/3007334/4415493

The Dictionary type makes no attempt to protect against the user modifying the key used. It is purely left up to the developer to be responsible in not mutating the key.

If you think about this a bit this is really the only sane route that Dictionary can take. Consider the implication of doing an operation like a memberwise clone on the object. In order to be thorough you'd need to do a deep clone because it will be possible for an object referenced in the key to also be mutated and hence affect the hash code. So now every key used in the table has it's full object graph cloned in order to protect against mutation. This would be both buggy and possibly a very expensive operation.

JamesFaix
  • 8,050
  • 9
  • 37
  • 73
1

Your dictionary value is a Point object. So, you can update the key with a new Point object.

Right Way (Update):
            d["a"]= new Point(5, 20);
            PrintPoint(d["a"]);
            Console.WriteLine(d["a"].x.ToString());

Round About: (Remove and Add)
            d.Remove("a");
            d.Add("a", new Point(2, 20));
            PrintPoint(d["a"]);
            Console.WriteLine(d["a"].x.ToString()); 
MKP
  • 22
  • 3