Reputation: 9
I have to admit, I'm a little stuck on this equals method. I think I'm pretty close because the first JUnit test that we're given passes but when I make a test for one that should return false, I fail and it returns true. Can I get some assistance on this, please?
public class Matrix {
// the dimensions of the matrix
private int numRows;
private int numColumns;
// the internal storage for the matrix elements
private int data[][];
/**
* @param d - the raw 2D array containing the initial values for the Matrix.
*/
public Matrix(int d[][])
{
// d.length is the number of 1D arrays in the 2D array
numRows = d.length;
if(numRows == 0)
numColumns = 0;
else
numColumns = d[0].length; // d[0] is the first 1D array
// create a new matrix to hold the data
data = new int[numRows][numColumns];
// copy the data over
for(int i=0; i < numRows; i++)
for(int j=0; j < numColumns; j++)
data[i][j] = d[i][j];
}
/**
* Determines whether this Matrix is equal to another object.
* @param o - the other object to compare to, which may not be a Matrix
*/
@Override // instruct the compiler that we intend for this method to override the superclass' (Object) version
public boolean equals(Object o)
{
// make sure the Object we're comparing to is a Matrix
if(!(o instanceof Matrix))
return false;
// if the above was not true, we know it's safe to treat 'o' as a Matrix
Matrix m = (Matrix)o;
/*
* TODO: replace the below return statement with the correct code.
*
* Returns true if this Matrix is equal to the input Matrix; returns false otherwise
*/
boolean matches = true;
if(o instanceof Matrix)
for(int i = 0; i < data[i].length; i++)
for(int j = 0; j < data[j].length; j++)
if(o[i][j] != m[i][j])
matches = false;
return matches;
}
Upvotes: 0
Views: 144
Reputation: 1251
If I were a teacher, these are the things I would point out, step-by-step:
This won't compile: if(o[i][j] != m[i][j])
Neither o
nor m
is an array, and so can't have subscripts.
Suppose we make this change: if (o.data[i][j] != m.data[i][j])
. Now o.data[i][j]
won't compile, because o
is an Object
type, which has no member named data
.
We could use a class cast like this: if (((Matrix) o).data[i][j] != m.data[i][j])
. Now, it will compile. But, the code above has this line: Matrix m = (Matrix)o;
So, o
and m
point to the same object. As long as that's the case, ((Matrix) o).data[i][j] == m.data[i][j]
will always be true
.
To fix the above, we need to use two different instances of Matrix
. We get one instance from the parameter: o
. The second one is implicit, but has a name: this
. So, this is what we want for our comparison: if(this.data[i][j] != m.data[i][j])
Here are some other comments, outside the scope of your question. You should be able to copy the segments of code, and put them together for a complete solution for a .equals
.
In general, it is a good idea to begin .equals
methods with 3 quick tests, like in the following:
@Override
public boolean equals (Object o) {
if (o == this) return true;
if (o == null) return false;
if(!(o instanceof Matrix)) return false;
Matrix m = (Matrix)o;
If this
and o
refer to the same object, .equals
should be guaranteed to return true
. In addition to being a quick test, the second will guard against a NullPointerException
later on in the code if someone passes a null
reference. If it passes all three tests, you can use the class cast and proceed.
When overriding .equals
, it's strongly recommended that hascode ()
be overridden also. So, please think about the following line:
if (m.hashCode() != this.hashCode()) return false;
If .hashcode ()
and .equals
are properly implemented, two objects returning different hash codes are guaranteed to return false
for .equals
. However, two objects returning the same hash code could return either true
or false
for .equals
. So, if the hascode()
test returns true
, we keep testing:
// two matrices can have different sizes. Comparing the sizes early
// will will guard against throwing
// an `ArrayIndexOutOfBoundsException` when the sizes are different
if ( m.numRows != this.numRows
|| m.numColumns != this.numColumns) return false;
Once you have determined that the contents of the two instances of Matrix
do not match, you can escape the for
loops with break
. It could save time if you are processing large matrices.
boolean matches = true;
outer: for(int i = 0; i < numRows; i++) {
for(int j = 0; numColumns; j++) {
if(this.data[i][j] != m.data[i][j]) {
matches = false;
if (!matches)
break outer;
}
}
}
return matches;
}
Upvotes: 1
Reputation:
The problem is here:
if(o instanceof Matrix)
for(int i = 0; i < data[i].length; i++)
for(int j = 0; i < data[j].length; j++)
if(o[i][j] == m[i][j])
return true;
You're looping over every element in the array and when you found any element that matches with the element in the other array, you're returning true
. I would create a boolean
(initially set to true
) and when I found an element that doesn't match, I would set the boolean
to false
and return the boolean
at the end of the method:
boolean matches = true;
if(o instanceof Matrix)
for(int i = 0; i < data[i].length; i++)
for(int j = 0; i < data[j].length; j++)
if(o[i][j] != m[i][j])
matches = false;
return matches;
Upvotes: 0