c++class

Getter method in matrix class not working properly


I'm trying to get this Getter method to return the assigned value in a matrix for a given row and column, and of course I set the return type as float. But when I'm running a test function, I'm getting back the number as an int, i.e 1 instead of 1.0, which is making my code not working probably, here's my getter and setter

float Matrix::element(int row, int col) const {
    if (row < 0 || row > nRows || col < 0 || col > nCols) 
        throw out_of_range("Index out of range.");
    return data[row * nCols + col];
}


void Matrix::set_element(int row, int col, float val) {
    if (row < 0 || row > nRows || col < 0 || col > nCols) {
        cerr << "Error: index is out of range. " << endl;
        throw out_of_range("Index is out of range.");
    }
    data[row * nCols + col] = val;
}

and here is the test function

void test() {
  Matrix A(2, 2);
  A.set_element(1, 1, 1.0);
  A.set_element(1, 2, 2.0);
  A.set_element(2, 1, 3.0);
  A.set_element(2, 2, 4.0);
  Matrix B(2, 2);
  A.add(B);
  cout << A.element(1, 1);                      // This is printing 1 instead of 1.0
  if (almost_equal(A.element(1, 1), 1.0)
      && almost_equal(A.element(1, 2),  2.0)
      && almost_equal(A.element(2, 1),  3.0)
      && almost_equal(A.element(2, 2),  4.0)) {
    cerr << "Passed" << endl;;
  } else {
    cerr << "Failed" << endl;
    return;
  }
}

I clearly did something wrong but can't find where, a second eye might help!:)


Solution

  • This code:

    void Matrix::set_element(int row, int col, float val) {
        if (row < 0 || row > nRows || col < 0 || col > nCols) {
            cerr << "Error: index is out of range. " << endl;
            throw out_of_range("Index is out of range.");
        }
        data[row * nCols + col] = val;
    }
    

    last line assumes indexing form 0 to nRows - 1 (or nCols - 1), but range checking is invalid. It accepts also index nRow.

    Now assuming your data have size nRow * nCols your test code is leading to buffer overflow, since it is using indexes from 1. So this line:

    A.set_element(2, 2, 4.0);
    

    will buffer overflow and your earlier check in Matrix::set_element will not catch that.

    So you have bug in production code (wrong checking of index ranges) and in test code.