Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
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;

    }




}

Hi, can anyone review my addMatrix method to see if I am following the instruction under correctly? This is the instructions:

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.

share|improve this question

2 Answers

up vote 3 down vote accepted

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.

share|improve this answer

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 constrauct and enw 1x1 DoubleMatrix.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.