Search code examples
javamatriximmutabilitypublic-method

Java Class variables are not immutable despite restricting access


I have a matrix object that I have written. I have intended that the matrix is immutable as an attribute of the object, implemented with a mutlidimensional array. as this.matrix. When I call the matrix methods. it is made sure to return a new matrix object, the array that is passed in to create the object should be deep copied.

I am failing one test in codewars, where a different double value is expected than the one given when I call the toArray() method. but I do not know any mroe information as the testing code is restricted.

can anyone see in the following code if there are any points in which I have created a Matrix whose this.matrix attribute can be modified from outside the object itself?

I have tried to use Arrays.copyOf in the constructor to make sure that new objects are created for the this.matrix attribute. I have made sure to return a new Matrix object for each method. so I do not really understand where else the 'this.matrix' instance variable could be modified unintentionally.

    import java.util.Arrays;

@SuppressWarnings("WeakerAccess")
public class Matrix {

    private double[][] matrix;
    private int  rows;
    private int columns;
    //constructor for the already sorted array

    Matrix(double[][] elements) {

        if (elements == null) {
            throw new IllegalArgumentException("Elements cannot be null");
        }
        int columns = elements[0].length;
        for(double[] element: elements){
            if(element == null){
                throw new IllegalArgumentException("Element of 2D Array cannot be null");
            }
            if(element.length != columns){
                throw new IllegalArgumentException("Array rows are not of equal length");
            }

        }

        this.matrix = elements;
        this.rows = this.matrix.length;
        this.columns = columns;


    }

    /// given row_length, row_column
    /// given list of elements
     Matrix(int rows, int columns, double... elements) {
        // remember double   ... elements means varargs

        if(elements == null){
            throw new IllegalArgumentException("Elements cannot be null");
        }

        if (elements.length != rows * columns) {
            throw new IllegalArgumentException("Illegal number of rows and columns for elements given");
        }
        this.rows = rows;
        this.columns = columns;
        this.matrix = new double[this.rows][this.columns];
        for(int i = 0; i<this.rows; i++){
//            System.arraycopy(elements, i*columns, this.matrix[i], 0, columns);
              double[] row = Arrays.copyOfRange(elements, i*columns, (i+1) * columns);
              this.matrix[i] = Arrays.copyOf(row,columns);
        }

    }

    public double[][] toArray() {
        return this.matrix;
        ///prints out the array to string
    }

    public Matrix multiply(double scalar){

        // this will just multiply the matrix with the scalar
        double[][] result = new double[this.rows][this.columns];

        for(int i = 0; i < this.matrix.length; i++){
            for(int j = 0; j < this.matrix[0].length; j++){

                result[i][j] = this.matrix[i][j] * scalar;

            }
        }
        return new Matrix(result);
    }

    public Matrix multiply(Matrix right){

        double[][] right_mat = right.toArray();
        //assert that left n = right m
        if(this.columns != right.rows){
            throw new IllegalArgumentException("Left matrix columns is not equal to Right matrix rows");
        }
        double[][] result = new double[this.rows][right.columns];


        //loop through twice and incrememnt the additions

        for(int m = 0; m < this.rows; m++){

            for(int k = 0; k < right.columns;k++){

                for(int n = 0; n < right.rows; n++){

                    result[m][k] += this.matrix[m][n] * right_mat[n][k];
                }
            }
        }

        return new Matrix(result);
    }

    public Matrix transpose(){
        double[][] result = new double[this.columns][this.rows];

        for(int i = 0; i < this.matrix[0].length; i++){

            final int column = i;
            // new_row should be column of existing
            double[] new_row = Arrays.stream(this.matrix).mapToDouble(doubles -> doubles[column]).toArray();
            result[i] = new_row;

        }
        return new Matrix(result);
    }
    public Matrix add(Matrix b){
        ///takes in Matrix adds to this one and
        ///returns the resulting Matrix
        if(this.columns != b.columns || this.rows != b.rows){
            throw new IllegalArgumentException("Matrices are not the same shape");
        }
        double[][] b_matr = b.toArray();
        double[][] result = new double[this.rows][this.columns];

        ///Matrix needs to have the same number of rows and columns

        for(int i= 0; i < this.rows; i++){
            for(int j = 0; j < this.columns; j++){
                result[i][j] = this.matrix[i][j] + b_matr[i][j];
            }
        }
        return new Matrix(result);
    }
}

Solution

  • First, your constructor Matrix(double[][] array) doesn't do deep copy of elements. Second, your toArray() method should return deep copy of this.matrix not that property itself.

    You can do deep copy of array like this

    double[][] copy = new double[this.matrix.length][];
    for (int i = 0; i < copy.length; ++i) {
      copy[i] = new double[this.matrix[i].length];
      for (int j = 0; j < copy[i].length; ++j) {
        copy[i][j] = this.matrix[i][j];
      }
    }