Can anyone review my addMatrix() method to see if I am following the instructions correctly?

These are the instructions:

This is a public method (I'm calling it addMatrix()) that has only one parameter for a DoubleMatrix to add this doubMatrix (not changing this doubMatrix) and the parameter's doubMatrix and return a new DoubleMatrix (you'll need a local 2-dim. array to store the result of adding and pass to the constructor).

Make sure you check if the dimensions of this doubMatrix and the parameter's doubMatrix are the same (if not, return a new DoubleMatrix calling the first constructor passing 1, 1).

I think I wrote the part where it said calling first constructor passing 1,1 wrong.

package homework3;

public class DoubleMatrix
{
private double[][] doubMatrix;

public DoubleMatrix()
{
int row;
int col;
if(row > 0 && col > 0)
{
makeDoubMatrix(1,1);
}
else
{
row = 1;
col = 1;
}
}

public DoubleMatrix(double[][] tempArray)
{
if(tempArray != null)
{
for(int i = 0; i < tempArray.length-1;i++)
{
if(tempArray[i].length == tempArray[i+1].length)
{
doubMatrix = tempArray;
}
}
}
else
{
makeDoubMatrix(1,1);
}
}

public int getDim1()
{
return doubMatrix.length;
}

public int getDim2()
{
return doubMatrix[0].length;
}

private void makeDoubMatrix(int row, int col)
{
double[][] tempArray  = new double[row][col];
for(int i = 0;i < tempArray.length;i++)
for(int j = 0;j < tempArray[i].length;j++)
{
tempArray[i][j] = Math.random() * (100);
} //end for
tempArray = doubMatrix;
}

public double[][] addMatrix(double[][] doubMatrix)
{
this. doubMatrix = doubMatrix;
double[][] tempArray = null;
if(this.doubMatrix.length == doubMatrix.length)
if(this.doubMatrix[0].length == doubMatrix[0].length)
{
for(int i = 0; i< this.doubMatrix.length;i++)
for(int j = 0; j< this.doubMatrix[i].length;j++ )
{
tempArray[i][j] = this.doubMatrix[i][j] + doubMatrix[i][j];// add two matrices
}//end for
}
else
{
return  tempArray = new double[1][1];
}

return tempArray;
}
}


For clarity, I would combine the size check conditional, and further, move it to it's own method to make the code a bit more clear. Working with objects (as @Alex suggested), will make this much easier:

if(this.doubMatrix.length == doubMatrix.length)
if(this.doubMatrix[0].length == doubMatrix[0].length) {
// stuff
}


Change to something like:

if(this.isSameSize(doubMatrix))
{
//stuff
}

// ...
// later
public boolean isSameSize(DoubleMatrix doub) {
return // Comparison of column and rows of backing arrays
}

• Also in your addMatrix() function, it looks like you are making some confusing/improper assignments:

this.doubMatrix = doubMatrix


...is overwriting your current DoubleMatrix backing array with the one you are passing into the method (the this keyword refers to the class' scope, not the method's scope). I would suggest simply renaming the addMatrix() parameter to something else so you don't have the confusion of working with this.doubMatrix and doubMatrix.

• Your addMatrix() result array (tempArray) is also initialized to null and never properly initialized before you start placing values there. That's going to throw a NullPointerException.

1. I think you should operate your matrices as objects, not as two-dimensional arrays, so you could change your method

public double[][] addMatrix(double[][] doubMatrix)


to

public DoubleMatrix add(DoubleMatrix secondMatrix)


and use the object for further operations.

2. Then your method

makeDoubleMatrix(int row, int col)


could be just a constructor:

public DoubleMatrix(int row, int col)

3. I really can't understand the purpose of the following code:

int row;
int col;
if(row > 0 && col > 0)


if you have no matrix params defined so:

public DoubleMatrix(){
this(1,1);
}


assuming that you will transform your makeDoubleMatrix() to constructor.

4. After all this your failed dimension check could just return new DoubleMatrix() which will construct and new a 1x1 DoubleMatrix.