конструктор 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

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

Прямо сейчас, ваш конструктор копирования не делает глубокую копию. Я также рекомендую использовать идиому копирования и обмена всякий раз, когда вы реализуете «Большую тройку». * На самом деле ваш operator= неверен.

---

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

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

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

Answer 2

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

2.Ваш оператор присваивания не должен просто копировать указатель, потому что это оставляет несколько Matrix объектов с одним и тем же указателем data, что означает, что указатель будет delete d несколько раз.Вместо этого вы должны создать глубокую копию матрицы.См. этот вопрос об идиоме копирования и обмена , в которой @GMan дает подробное объяснение о том, как написать эффективную, безопасную для исключения функцию operator=.

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

Answer 3

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

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 4

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

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

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

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

РЕДАКТИРОВАТЬ: Также исправьте вещи, упомянутые в других ответах.Это тоже важно.Но если вы измените конструктор копирования и операторы присваивания для выполнения глубоких копий, то не «новые» объекты при заполнении вектора, иначе это приведет к утечке памяти.

Answer 5

Вы используете new[], но вы не используете delete[]. Это действительно плохая идея .

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

И, да, у вас также отсутствует конструктор копирования . Вот о чем Правило Трех .

Answer 6

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

Редактировать: рискуя редактировать, то, что вы разместили, является в основном постером, объясняющим, почему вы должны использовать стандартный контейнер вместо того, чтобы писать свой собственный. Если вам нужна прямоугольная матрица, попробуйте написать ее как обертку вокруг вектора .