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);
}
}
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];
}
}