Ошибка неверного доступа в 2D-массиве (матрицы) - PullRequest
1 голос
/ 05 октября 2011

У меня есть небольшая проблема ... Я понимаю, что такое ошибка EXC_BAD_ACCESS, и я, как правило, знаю, как ее исправить, но эта ошибка полностью наполнила меня. У меня есть все это в классе, вот один метод:

double Matrix::get_element(int r, int c) const {
    //Retrieve the element at row r and column c
    //Should not modify the value stored in Matrix but return a double copy of the value

    double currentValue = matrix[r][c];
    return currentValue;
}

Теперь у меня есть другой фрагмент кода, который вызывает этот метод:

std::string Matrix::to_string() const {
    std::string result;
    double current;
    Matrix working = *this;
    std::ostringstream oss;

    oss << "[";
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < cols; j++) {
            current = 0.0;
            current = working.get_element(i, j);
            oss << " " << current << " ";
        }
        oss << "; ";
    }
    oss << "]";
    result = oss.str();
    return result;
}

Я знаю, что рабочий объект имеет 3 строки и 3 столбца в точке, где вызывается working.get_element(i, j);. Список переменных показывает мне как раз перед методом get_element(), что для строк и столбцов установлено значение 3. В этом методе я могу получить значение get_element(0, 0), но не get_element(0, 1).

Я не понимаю, почему это так ... Кто-нибудь знает, почему или требует больше моего кода, чтобы понять, почему эти методы вызываются?

EDIT: Вот заголовочный файл:

class Matrix {
private:
    //Any variables required
    int rows;
    int cols;
    double **matrix;

public:
    Matrix();   //Working M
    ~Matrix();  //Working M
    Matrix(int r, int c);   //Working M

    int getRows();
    int getCols();

    void set_element(int r, int c, double val); //Working M
    double get_element(int r, int c) const; //Working M

    void clear(); //Working M        
    bool is_empty(); //Working M         
    bool is_identity(); //Working M

    const Matrix transpose(); //Working M
    int minorMat(double **dest, const int row, const int col, int order); //Working M
    double get_determinent(); //Working M
    double higherDeterminents(int order); //Working M

    const Matrix operator+(const Matrix &rhs); //Working M       
    const Matrix operator-(const Matrix &rhs); //Working M    
    const Matrix operator*(const Matrix &rhs); 
    bool operator==(const Matrix &rhs); //NOT assessed
    const Matrix operator*(const double &rhs);        
    const Matrix operator/(const double &rhs);
    Matrix & operator=(const Matrix &rhs);

    std::string to_string() const;
};

Игнорируйте комментарии, извините. А это конструкторы / деструкторы:

Matrix::Matrix() {
    //Basic Constructor
    rows = 1;
    cols = 1;
    matrix = new double*[rows];
    for (int i = 0; i < rows; ++i) {
        matrix[i] = new double[cols];
    }
}

Matrix::~Matrix() {
    //Basic Deconstructor
    for (int i = 0; i < rows; ++i) {
        delete[] matrix[i];
    }
    delete[] matrix;
    rows = NULL;
    cols = NULL;
    matrix = NULL;
}

Matrix::Matrix(int r, int c) {
    //Empty matrix (all 0's) with r rows and c columns, if they are -ve, set to 1
    rows = r;
    cols = c;

    if (cols < 0)
        cols = 1;
    if (rows < 0)
        rows = 1;

    matrix = NULL;
    matrix = new double*[rows];
    for (int i = 0; i < rows; i++) {
        matrix[i] = new double[cols];
    }
}

EDIT2:

Matrix & Matrix::operator=(const Matrix &rhs) {
    //rhs is matrix to be copied
    //rhs compied into Matrix called on
    double toCopy;
    for (int i = 0; i < rhs.rows; i++) {
        for (int j = 0; j < rhs.cols; j++) {
            toCopy = rhs.get_element(i, j);
            this->set_element(i, j, toCopy);
        }
    }
    return *this;
}

Ответы [ 3 ]

1 голос
/ 05 октября 2011

Ваш класс нарушает правило " big three ". Если в классе есть один из деструктора, оператора присваивания или конструктора копирования, то, скорее всего, вам нужно иметь все три.

В вашем случае у вас есть деструктор, но нет оператора присваивания или конструктора копирования, и это создаст условие UB, когда вы выполните Matrix working = *this.

Кстати, есть еще две проблемы с вашим кодом

  1. Из комментария кажется, что вы думаете, что new double[size] инициализирует элементы в 0, и это не так.

  2. Большая часть вашего кода технически очень плохая. Ваш класс матрицы было бы намного проще (без кода) реализовать правильно, используя std::vector вместо указателей и динамической памяти. Конечно, если это просто упражнение, то имеет смысл избегать использования std::vector.

Кстати, если вы никогда не слышали о правиле "большой тройки", скорее всего, вы пытаетесь изучать C ++ с помощью экспериментов, а не путем чтения.

С C ++ это не умный ход ... логику можно использовать в качестве замены для изучения, если 1) тема очень логична, 2) если вам могут сказать, когда вы ошибаетесь.

Вместо этого C ++ очень очень сложен и в некоторых местах также довольно нелогичен (по историческим причинам), поэтому есть части, в которых логика просто вводит вас в заблуждение.

Более того, когда вы делаете ошибку в C ++, вы, как правило, получаете не сообщение об ошибке, а "Undefined Behavior". Это в основном делает очень трудным изучение C ++ с помощью экспериментов, потому что даже неправильный код может , по-видимому, работать. Кроме того, очень легко написать код, который выглядит хорошо, а вместо этого совершенно неверно по незаметным причинам.

Вместо того, чтобы просто экспериментировать, вы должны взять хорошую книгу и прочитать ее от корки до корки ...

1 голос
/ 05 октября 2011

Ваш Matrix(int r, int c) выделяет память для matrix, но оставляет значения, на которые он указывает, неинициализированными.Добавьте что-то вроде этого в конструктор:

for (int i = 0; i < r; i++) {
        for (int j = 0; j < c; j++) {
            this->set_element(i, j, 0);
        }
    }

Делая так:

int main()
{
    Matrix m(3,3);
    std::cout << m.to_string();
}

Вывод:

[ 0 0 0; 0 0 0; 0 0 0; ].

То же самоев конструкторе по умолчанию:

Matrix::Matrix() {
    //Basic Constructor
    rows = 1;
    cols = 1;
    matrix = new double*[rows];
    for (int i = 0; i < rows; ++i) {
        matrix[i] = new double[cols];
    }
}

Вы выделили память, но, куда указывает матрица [0] [0], это неинициализированное значение мусора.Сделайте что-то вроде matrix[0][0] = 0; или любое другое значение по умолчанию, которое вы хотите.

Надеюсь, это поможет.

1 голос
/ 05 октября 2011

Мы не можем сказать, когда вы не указали, как объявляете и инициализируете элемент matrix.Использование чего-то подобного в вашем CTOR должно быть хорошо:

class Matrix {
   float matrix[3][3];
   ...
}

Не забудьте инициализировать это в вашем CTOR чем-то, что имеет смысл.

Кстати: зачем ты это делаешь: Matrix working = *this; ??Вместо этого вы можете просто this->get_element(i, j);, что не приведет к копированию всего вашего объекта.[1]

РЕДАКТИРОВАТЬ: Обновление после обновления вашего ответа.Вы должны быть осторожны с вашими копиями CTOR и operator=() утверждениями.Можно легко сделать двойное удаление или что-то безобразное.

EDIT2: Я думаю, что проблема в этой строке:

Matrix working = *this;

Вы создаете новыйкопия working вашего this объекта.Но working инициализируется только с 1 столбцом и 1 строкой (как определено в вашем стандартном CTOR).Я не уверен, проверяете ли вы границы при вызове set_element или get_element, поэтому я думаю, что вы пишете за пределами своих массивов.

Я думаю, что лучшая идея - просто удалить строкуMatrix working = *this; и придерживаться моего совета выше: this->get_element(i, j); в std::string Matrix::to_string() const.

...