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 itaddMatrix()
) that has only one parameter for aDoubleMatrix
to add thisdoubMatrix
(not changing thisdoubMatrix
) and the parameter'sdoubMatrix
and return a newDoubleMatrix
(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'sdoubMatrix
are the same (if not, return a newDoubleMatrix
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;
}
}
2 Answers 2
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 (thethis
keyword refers to the class' scope, not the method's scope). I would suggest simply renaming theaddMatrix()
parameter to something else so you don't have the confusion of working withthis.doubMatrix
anddoubMatrix
.Your
addMatrix()
result array (tempArray
) is also initialized tonull
and never properly initialized before you start placing values there. That's going to throw aNullPointerException
.
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.
Then your method
makeDoubleMatrix(int row, int col)
could be just a constructor:
public DoubleMatrix(int row, int col)
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.After all this your failed dimension check could just return
new DoubleMatrix()
which will construct and new a 1x1DoubleMatrix
.