C++ конструктор с новым

Question

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

class Matrix 
{ 
    public: 
    Matrix(int w,int h) : width(w),height(h) 
    {   
     data = new unsigned char[width*height]; 
    } 

    ~Matrix() { delete data; } 

    Matrix& Matrix::operator=(const Matrix&p) 
    { 
      width = p.width; 
      height = p.height; 
      data= p.data; 
      return *this; 
    } 
    int width,height; 
    unsigned char *data; 
} 

......... 
// main code 
std::vector<Matrix> some_data; 

for (int i=0;i<N;i++) { 
    some_data.push_back(Matrix(100,100)); // all Matrix.data pointers are the same 
} 

Когда я заполняю вектор экземплярами класса, внутренние указатели данных все указывают на одну и ту же память?

Answer 1

1. Вам не хватает конструктора копирования.

2. Ваш оператор присваивания не следует просто скопировать указатель, потому что оставляет множественные Matrix объекты с одинаковыми data указателем, что означает, что указатель будет delete г несколько раз. Вместо этого вы должны создать глубокую копию матрицы. См. this question about the copy-and-swap idiom, в котором @GMan дает подробное объяснение того, как написать эффективную, исключающую исключение функцию operator=.

3. Вам необходимо использовать delete[] в вашем деструкторе, а не delete.

Answer 2

Вы пропустили конструктор копирования.

Matrix(const Matrix& other) : width(other.w),height(other.h) 
{   
    data = new unsigned char[width*height]; 
    std::copy(other.data, other.data + width*height, data); 
} 

Редактировать: И ваш деструктор ошибочен. Вы должны использовать delete[] вместо delete. Также оператор присваивания просто копирует адрес уже выделенного массива и не выполняет глубокую копию.

Answer 3

Всякий раз, когда вы пишете один из конструктора копирования, оператора копирования или деструктора, вы должны сделать все три. Это «Большая тройка», а предыдущее правило - «Правило трех».

Прямо сейчас, ваш экземпляр-копирайтер не делает глубокую копию. Я также рекомендую вам использовать copy-and-swap idiom всякий раз, когда вы реализуете The Big Three. * В его нынешнем виде ваш operator= неверен.

---

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

Этот класс утилиты должен будет реализовать The Big Three, но пользовательскому классу фактически не нужно будет реализовывать ни одно из них, потому что неявно сгенерированные будут обработаны должным образом благодаря классу утилиты.

Конечно, такой класс уже существует как std::vector.

Answer 4

Ваш недостающий экземпляр ctor уже указан. Когда вы это исправите, у вас все еще будет серьезная проблема: оператор присваивания выполняет мелкую копию, что даст неопределенное поведение (дважды удаляя одни и те же данные). Вам нужна либо глубокая копия (т. Е. В вашем operator= выделить новое пространство, скопировать существующее содержимое в новое пространство), либо использовать что-то вроде подсчета ссылок, чтобы гарантировать, что данные будут удалены только один раз, когда последняя ссылка на него будет уничтожена.

Редактировать: из-за риска переделать то, что вы опубликовали, в основном является плакатом-ребенком для того, почему вы должны использовать стандартный контейнер вместо того, чтобы писать свой собственный. Если вы хотите прямоугольную матрицу, рассмотрите ее как wrapper around a vector.

Answer 5

Вы используете new[], но вы не используете delete[]. Это really bad idea.

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

И да, вы Отсутствует копия конструктора, тоже. Вот что такое Rule of Three.

Answer 6

Проблема в том, что вы создаете временное сообщение с Matrix(100,100), которое разрушается после того, как оно неглубокое копировано в вектор. Затем на следующей итерации он строится снова, и одна и та же память выделяется для следующего временного объекта.

Чтобы это исправить:

some_data.push_back(new Matrix(100,100)); 

Вы также должны добавить некоторый код, чтобы удалить объекты в матрице, когда вы закончите.

EDIT: Также исправьте материал, упомянутый в других ответах. Это тоже важно. Но если вы измените свой конструктор копий и операторы присваивания для выполнения глубоких копий, не делайте «новых» объектов при заполнении вектора или утечки памяти.