Как мне реализовать конструктор копирования и оператор присваивания для матричного класса?
У меня есть матричный класс с такими полями:
template <typename T>
class Matrix
{
private:
T **matrix = nullptr;
int rows;
int cols;
На этом этапе я написал оператор присваивания и конструктор копирования. Но, во-первых, есть дублирование кода, как его избежать, а во-вторых, они мне кажутся очень похожими, как можно улучшить эти методы, чтобы они выглядели нормально?
Matrix(const Matrix &matrix_) : rows(matrix_.rows), cols(matrix_.cols)
{
matrix = static_cast<T **>(new T *[rows]);
for (int i = 0; i < rows; i++)
{
matrix[i] = static_cast<T *>(new T[cols]);
}
for (int i = 0; i < rows; i++)
{
for (int j = 0; j < cols; j++)
{
matrix[i][j] = matrix_[i][j];
}
}
}
Matrix &operator=(const Matrix &matrix_)
{
if (&matrix == this)
{
return *this;
}
clean();
rows = matrix_.rows;
cols = matrix_.cols;
matrix = static_cast<T **>(new T *[rows]);
for (int i = 0; i < rows; i++)
{
matrix[i] = static_cast<T *>(new T[cols]);
}
for (int i = 0; i < rows; i++)
{
for (int j = 0; j < cols; j++)
{
matrix[i][j] = matrix_[i][j];
}
}
}
void clean()
{
if (cols > 0)
{
for (int i = 0; i < rows; i++)
{
delete[] matrix[i];
}
}
if (rows > 0)
{
delete[] matrix;
}
}
По условию задания запрещено использовать STL-контейнеры, я должен сам реализовать элементы управления
Добавлена семантика перемещения
Matrix(Matrix &&other) noexcept : rows(std::move(other.rows)), cols(std::move(other.cols)), data(new T(rows * cols))
{
other.data = nullptr;
rows = 0;
cols = 0;
}
Matrix &operator=(Matrix &&other) noexcept
{
if (&other == this)
{
return *this;
}
if (rows != other.rows && cols != other.cols)
{
std::cout << "Error assigning matrices of different sizes" << std::endl;
exit(-1);
}
clean();
std::swap(data, other.data);
std::swap(rows, other.rows);
std::swap(cols, other.cols);
return *this;
}
Вам действительно нужно менять столбцы и строки во время выполнения? Если нет, рассмотрите возможность их объявления в параметрах шаблона и используйте std::array, вам не понадобится настраивать конструктор копирования и оператор присваивания.
Есть ли у вас причина использовать ручное управление памятью в классе Matrix? Это какое-то задание (без каламбура) или что-то в этом роде? Если нет, то предложение @ted имеет много достоинств.
Как и @ 273K, если на то пошло, но по другим причинам.
Да, это задача. Преподаватель запретил использовать контейнеры из STL, приходится самому реализовывать управление ресурсами
Хорошо, тогда используйте указатель один. Вы даже можете создать небольшой класс интеллектуальных указателей для управления памятью.
Почему один, а не двойной? А как насчет оператора присваивания и конструктора присваивания. Они должны быть очень похожи (включая дублирование кода)
Ваш конструктор копирования может вызвать ваш оператор присваивания (как operator=), если вы хотите, и если предварительные условия верны. В этом помогает инициализация ваших переменных-членов в момент их объявления.
@ЕлизаветаТараненко Пожалуйста, проверьте мой ответ. Я думаю, это то, что вы хотите. Я отредактировал, чтобы использовать ручное управление памятью вместо std::vector Конечно, понадобятся конструкторы и деструкторы...
Не похоже, что эти static_cast нужны. Приведения должны заставить вас заподозрить, что вы делаете что-то не так.
Указана ли в присваивании семантика присваивания матриц для матриц разной размерности?
@einpoklum, ты имеешь в виду не только квадратные матрицы? Если так, то да, матрицы могут быть разного размера
@ЕлизаветаТараненко: Я имею в виду, что должно произойти, если вы присвоите матрице размера 10x20 матрицу размера 1x1? Это должно потерпеть неудачу, или целевая матрица теперь должна стать матрицей 10x20?
@ЕлизаветаТараненко Что должно произойти, если вы назначите матрицу 2x3 матрице 3x2?
Я еще не обработал эту ошибку, но я думаю, что ошибка должна быть поднята
Зачем вам использовать семантику перемещения с примитивами? Это не имеет никакого смысла.
Если вам требуются матричные назначения только между значениями с одинаковым количеством строк/столбцов, это сделает такие вещи, как помещение их в вектор, непрактичным, если только каждая матрица, помещенная в вектор, не имеет одинакового размера. Вы можете сделать размеры неизменными, поместив строки и столбцы в шаблон. Тогда все будет работать, так как разные размеры — это разные классы.
@Doug Этого недостаточно. Также требуется знать размер матрицы во время компиляции...
Существует явный сайт для проверки кода codereview.stackexchange.comНО обратите внимание, что сначала код должен работать. Поэтому, если вы можете протестировать свой код с помощью модульных тестов, и он работает должным образом, вы можете проверить его там. Хотя вы, вероятно, захотите просмотреть весь класс, а не только его часть.
@simre Меня беспокоило то, что без возможности назначать матрицы с разными строками / столбцами такие вещи, как std::swap(mat1, mat2);, не будут работать. И это используется вектором.
Ответы 2
Тогда используйте одномерный массив. Это будет намного чище, проще и быстрее, чем массив указателей на массивы... Вы можете сделать что-то вроде:
template <typename T>
class Matrix
{
private:
// Note: Assuming T is a trivial type, most likely a fundamental type...
T* data; // Flattened matrix with size = rows*cols. Be careful,
// for really big matrices this multiplication could overflow!!!
// You can use unsigned type since negative size is a nonsense...
unsigned int rows;
unsigned int cols;
public:
Matrix(const Matrix& other)
: data(new T[other.rows*other.cols])
, rows(other.rows)
, cols(other.cols)
{
/// You can do this:
/// for(int i = 0; i < rows*cols; ++i)
/// data[i] = other.data[i];
/// or simply call the copy assign operator:
operator=(other);
}
Matrix& operator=(const Matrix& other)
{
// This is only for matrix with the same dimensions.
// Delete and allocate new array if the dimensions are different.
// This is up to the OP if he can have differently sized matrices or not...
// Also assert will "disappear" if NDEBUG (ie. in release) is defined.
assert(rows == other.rows);
assert(cols == other.cols);
for(int i = 0; i < rows*cols; ++i)
data[i] = other.data[i];
return *this;
}
};
Примечание. Вам также нужно будет определить конструктор (ы) и деструктор, конечно... Я уверен, что вы не можете сделать это намного проще, чем это...
Спасибо! Но у меня есть небольшой вопрос, поскольку у нас есть только один указатель, то функция очистки (для деструктора и, возможно, будущего оператора перемещения конструктора перемещения) будет выглядеть так: if (nullptr != data) delete[] data;
@ЕлизаветаТараненко Вам не нужно проверять, является ли это nullptr. Просто delete[] data; В назначении перемещения вы можете просто std::swap указатели двух контейнеров. Не надо delete[].
@ЕлизаветаТараненко delete[] ничего не будет делать на nullptr. Вы можете смело назвать это nullptr. @TedLyngmo Он/она не может использовать std::swap и я думаю...
@simre Вероятно, это правда, но понять, как это работает, и написать аналогичную функцию не так уж сложно.
@ЕлизаветаТараненко Для clean, если он очищает весь объект, вы также должны сбросить cols и rows на 0. Не оставляйте объект в недопустимом состоянии. Это может вызвать головную боль позже довольно легко ...
Ваш оператор = будет волей-неволей копировать из матрицы n x 2m в матрицу 2n x m.
@simre: Но это было просто указание на более глубокую проблему. Почему вы считаете, что матрицы разных размерностей не должны быть присваиваемыми, если размерности не являются константными элементами? ОП не использовал эту семантику.
@simre Просто имейте в виду, что assert удаляются в релизных сборках (если NDEBUG определено). Если размеры не совпадают и размер выделенной вами памяти не тот же (или, возможно, просто меньше), вы можете просто delete[] выделить старую выделенную память и выделить то, что необходимо для копирования другой Matrix
@einpoklum Если вы переместите матрицу 2x3 в матрицу 3x2, то каким должно быть состояние элементов, отсутствующих в исходной матрице? Что делать с элементами, которым нет места в целевой матрице?
@GoswinvonBrederlow: вы бы изменили размеры и при необходимости перераспределили. Это означает, что вы также можете назначить матрицу 10x20 матрице 1x1... конечно, это вопрос того, что было запрошено OP.
@einpoklum Изменение размеров также возможно, и да, тогда вам придется перераспределить данные. Я понимаю Матрицу как математический объект Матрицы, и тогда я полностью согласен с утверждениями. Для двумерного массива без математического смысла все может быть по-другому.
«Я в полном порядке» - но учителя ОП могут быть не такими. Так что, по крайней мере, ваш ответ должен пояснить, что здесь должен быть сделан явный выбор дизайна.
@simre: Твоя проблема в том, что ты думаешь как математик. Это StackExchange, сайт программирование, и OP получил назначение программирование. По вашей логике std::vector нельзя назначать друг другу, если их размеры различны.
Для полноты я добавил реализацию семантики перемещения (для меня это новый мир). Можете ли вы оценить и сказать, что не так и что можно исправить?
@ЕлизаветаТараненко В конструкторе перемещения вам не нужно move, так как вы имеете дело с фундаментальными типами и указателями. Также вам не нужно выделять память в конструкторе перемещения. Вы просто «воруете». Копируешь с другой Матрицы, а потом другую ставишь как сделал. При назначении перемещения, если clean() устанавливает nullptr, а cols, rows=0, то вроде нормально. Также вы можете проверить std::exchange, но это может запутать некоторых людей...
Не понимаю, почему у этого -4 голоса. Это может быть не идеально, но делает то, что просил ОП.
@MartinYork Никто не знает. :D Только те, кто пытался убедить меня реализовать логическую ошибку... :D
@simre: это не дает сильной гарантии исключения. Он не использует стандартный шаблон копирования и подкачки. Это очень неэффективно, потому что каждый элемент в массиве создается, а затем присваивается (что еще важнее, T нетривиален). Но он работает так, как ожидалось.
Как правило, если вам нужен общий код между методами, лучший способ — разделить общий код на (частные) вспомогательные методы. Например, у вас может быть метод alloc_, который имеет цикл, вызывающий new, и free_, который вызывает delete. Тогда вы могли бы:
Matrix(const Matrix &a) : rows(a.rows), cols(a.cols) {
alloc_();
copy_(a);
}
Matrix &operator=(const Matrix &a) {
free_();
rows = a.rows;
cols = a.cols;
alloc_();
copy_(a);
}
~Matrix() { free_(); }
Вам лучше использовать одномерный массив, к которому вы используете
T& operator()(size_t row, size_t col) { return matrix[row * cols + col]; }для доступа. Копирование одного из них, вероятно, настолько быстро, насколько это возможно. Используйтеvector<T>вместоmatrix, и вы получите большую часть бесплатно.