Trying to create immutable class - private set being ignored

Question

I'm trying to create an immutable class to represent a matrix. However, while the private setters in the Rows and Columns is working I am still able to modify the contents of Elements. How do I properly create an immutable class?

    var matrix = new Matrix(new double[,] {
                                          {1,1,1,1},
                                          {1,2,3,4},
                                          {4,3,2,1},
                                          {10,4,12, 6}});

    matrix.Elements[1,1] = 30; // This still works!

Matrix class:

   class Matrix
    {
        public uint Rows { get; private set; }
        public uint Columns { get; private set; }
        public double[,] Elements { get; private set; }

        public Matrix(double[,] elements)
        {
            this.Elements = elements;
            this.Columns = (uint)elements.GetLength(1);
            this.Rows = (uint)elements.GetLength(0);
        }
    }

Don't share your array with the outside world. It's not immutable. Instead use an indexer to grant read access. I'll provide an example in a few minutes. — nvoigt, Oct 07 '14 at 15:27
Instead of using a 2d array, I believe you will have to create your own class that wraps the 2d array with a `private set` indexer — clcto, Oct 07 '14 at 15:27
This is because once you get the matrix via the `Elements` property, you have the full array and are therefore able to read and write as you see fit. What I would do instead is to not expose the `Elements` property, but instead expose an overridden index operator `[,]` (as seen [here](http://stackoverflow.com/questions/287928/how-do-i-overload-the-square-bracket-operator-in-c) and only allow read access to it. — GEEF, Oct 07 '14 at 15:28
Make `Elements` a method instead of a property `public double GetElement(int row, int column)` — juharr, Oct 07 '14 at 15:28
Both these ideas sound good to me, which one would normally be used? — user9993, Oct 07 '14 at 15:28
"Normally used"? Depending on your goal... Indexers would produce more natural looking code, but either accessor is really optional if you provide complete set of operations (like `+`, `*`, and iterators for vectors by row/column). — Alexei Levenkov, Oct 07 '14 at 15:54

nvoigt · Answer 1 · 2014-10-07T16:12:06.240

An array is not immutable. You need to use an indexer to have an immutable type:

class Matrix
{
    public uint Rows { get; private set; }
    public uint Columns { get; private set; }
    private readonly double[,] elements;

    public Matrix(double[,] elements)
    {
        // this will leave you open to mutations of the array from whoever passed it to you
        this.elements = elements;

        // this would be perfectly immutable, for the price of an additional block of memory:
        // this.elements = (double[,])elements.Clone();

        this.Columns = (uint)elements.GetLength(1);
        this.Rows = (uint)elements.GetLength(0);
    }

    public double this[int x, int y]
    {
      get
      {
        return elements[x, y];
      }

      private set
      {
        elements[x, y] = value;
      }
    }
}

You can use this by using the indexer on your instance:

var matrix = new Matrix(new double[,] {
                                      {1,1,1,1},
                                      {1,2,3,4},
                                      {4,3,2,1},
                                      {10,4,12, 6}});

double d = matrix[1,1]; // This works, public getter

matrix[1,1] = d; // This does not compile, private setter

make sure to check Falanwe's answer about need for immutable array... Side note: `set`, even private looks very strange in "immutable" class. — Alexei Levenkov, Oct 07 '14 at 16:03
@AlexeiLevenkov thanks for the hint, added a comment in the code about that. — nvoigt, Oct 07 '14 at 16:13
Nice answer. I still dislike two little things: setters in a immutable class, even if they are private, leave you open to a future mistake during code maintenance, and underlying fields for Rows and Columns properties which are unneeded. — Falanwe, Oct 07 '14 at 19:01

score 3 · Answer 2 · answered Oct 07 '14 at 15:28

3

I wouldn't expose the array, but instead create an indexer:

public double this[int x, int y]
{
    get
    {
        return elements[x,y];
    }
}

You would then use the indexer like so:

matrix[1,1]

answered Oct 07 '14 at 15:28

Michael G

6,695
2
41
59

1

This is the best, no setter is needed because other member functions can directly write to `elements[x,y]` – Ben Voigt Oct 07 '14 at 15:33

score 3 · Answer 3 · answered Oct 07 '14 at 15:43

As others have pointed out, you can nearly achieve immutability by not exposing your inner Array and instead exposing an indexer to the outside world.

public class Matrix
{
    private readonly double[,] _elements;

    public uint Rows { get { return (uint)elements.GetLength(0);} }
    public uint Columns { get { return (uint)elements.GetLength(1);} }

    public Matrix(double[,] elements)
    {
        this._elements = elements;
    }

    public double this[int x, int y]
    {
      get
      {
        return elements[x, y];
      }
   }
}

Now you can access your elements easily:

matrix[1,2]

That nearly makes your class immutable. Why nearly? Because the inner array is still mutable, and it was passed to you by the caller, which may modify it later (by mistake or on purpose), defeating your immutability.

How to prevent that flaw?

The simplest way is to make a copy of the original array (with the Clone() method or otherwise, your pick), and store that copy. This way your inner array is never exposed to anything outside the class, achieving true external immutability (barring any reflexion or unsafe code, of course).

Another nice option for reaching immutability is to use elements from the namespace System.Collections.Immutable as your building blocks. Having already immutable collections helps immensely to implement more complex scenarios.

score 0 · Answer 4 · answered Oct 07 '14 at 15:31

Heres another way...

class Matrix
    {
        private uint Rows { get; private set; }
        private uint Columns { get; private set; }
        private double[,] Elements { get; private set; }

        public Matrix(double[,] elements)
        {
            this.Elements = elements;
            this.Columns = (uint)elements.GetLength(1);
            this.Rows = (uint)elements.GetLength(0);
        }

        public double GetElement(int row, int column)
        {
            return this.elements[row, column];
        }
    }

Make the array private and expose a method to retrieve the value.

Trying to create immutable class - private set being ignored

4 Answers4