Seda
Seda

Reputation: 103

C#: Immutable class

I have a class which should be immutable in this class i have only get indexer a private set property so why this is not immutable and i can set some field in array as you could see in main class...

class ImmutableMatice
{       
    public decimal[,] Array { get; private set; } // immutable Property

    public ImmutableMatice(decimal[,] array)
    {
        Array = array;
    }
    public decimal this[int index1, int index2]
    {
        get { return Array[index1, index2]; }
    }

........ and in main method if i fill this class with data and change the data

    static void Main(string[] args)
    {
        decimal[,] testData = new[,] {{1m, 2m}, {3m, 4m}};
        ImmutableMatice matrix = new ImmutableMatice(testData);
        Console.WriteLine(matrix[0,0]); // writes 1
        testData[0, 0] = 999;
        Console.WriteLine(matrix[0,0]); // writes 999 but i thought it should 
                                        // write 1 because class should be immutable?
    }
 }

Is there any way how to make this class immutable?

Ah yes the solution was copy array to new array in constructor like this:

    public ImmutableMatice(decimal[,] array)
    {
        decimal[,] _array = new decimal[array.GetLength(0),array.GetLength(1)];
        //var _array = new decimal[,] { };
        for (int i = 0; i < array.GetLength(0); i++)
        {
            for (int j = 0; j < array.GetLength(1); j++)
            {
                _array[i, j] = array[i, j];
            }
        }
        Array = _array;
    }

Upvotes: 1

Views: 217

Answers (2)

Jeroen Vannevel
Jeroen Vannevel

Reputation: 44439

Your class is immutable, but the objects inside it aren't.

Having public decimal[,] Array { get; private set; } will only guarantee that you cannot set the property Array to a new instance of Array, but it does not prevent you from accessing the existing object and changing its values (which aren't immutable).

You might want to look into the appropriately named ReadOnlyCollection<T> class.

As @Mike pointed out and I looked past the first time: there's a twist to this because you are accessing the value through the testData object and not through matrix. While the original point still stands, it is more exact to say that the problem you have is that you are changing values in the underlying object which has its reference passed around. You're bypassing the ImmutableMatice object alltogether.

The beforementioned solution of using a ReadOnlyCollection<T> still stands: by creating this read-only wrapper around it, you won't be able to change it anymore afterwards. Howver this is only the case when you actually use it the way its intended: through ImmutableMatice and not through the underlying collection which you still have a reference to.

Another solution that solves this problem is to copy the contents of the original array to another one to "disconnect" it from the array your still have a reference to.

In order to illustrate this, consider the following samples. The first one demonstrates how the underlying reference can still be influenced while the second one shows how it can be solved by copying your values to a new array.

void Main()
{
    var arr = new[] { 5 };
    var coll = new ReadOnlyCollection<int>(arr);
    Console.WriteLine (coll[0]); // 5
    arr[0] = 1;
    Console.WriteLine (coll[0]); // 1
}

void Main()
{
    var arr = new[] { 5 };
    var arr2 = new int[] { 0 };
    Array.Copy(arr, arr2, arr.Length);
    var coll = new ReadOnlyCollection<int>(arr2);
    Console.WriteLine (coll[0]); // 5
    arr[0] = 1;
    Console.WriteLine (coll[0]); // 5
}

Upvotes: 4

YoryeNathan
YoryeNathan

Reputation: 14512

That is because you are actually changing the data in the ARRAY, rather than the indexer.

static void Main(string[] args)
{
    decimal[,] testData = new[,] {{1m, 2m}, {3m, 4m}};
    ImmutableMatice matrix = new ImmutableMatice(testData);
    Console.WriteLine(matrix[0,0]); // writes 1
    testData[0, 0] = 999; // <--- THATS YOUR PROBLEM
    Console.WriteLine(matrix[0,0]); // writes 999 but i thought it should 
                                    // write 1 because class should be immutable?
}

You can copy the array into your private property in the constructor to avoid this situation.

Note that you indeed cannot write matrix[0,0] = 999; because the indexer has no setter.

Edit

As Chris pointed out (how could I have missed it myself?) - you shouldn't expose the array as a property at all (which means in most cases it doesn't even have to be a property).

Consider the following code instead:

private decimal[,] _myArray; // That's private stuff - can't go wrong there.

public decimal this[int index1, int index2]
{
    // If you only want to allow get data from the array, thats all you ever need
    get { return Array[index1, index2]; }
}

Upvotes: 4

Related Questions