2015-09-22 2 views
4

У меня есть программа, которая использует класс для динамического выделения массива. Я перегрузил операторов, которые выполняют операции над объектами этого класса.C++: Ошибка выполнения при использовании перегруженных операторов присваивания соединений

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

malloc: * error for object 0x7fd388500000: pointer being freed was not >allocated * set a breakpoint in malloc_error_break to debug

В частных переменных членах, я объявляю массив как это:

double* array_d; 

Я динамически выделить массив в перегруженном конструкторе:

Students::Students(int classLists) 
{ 
    classL = classLists; 
    array_d = new double[classL]; 
} 

у меня есть следующие две перегруженные конструкторы, определенные как друзья класса студентов:

friend Student operator+= (const Student&, const Student&); 
friend Student operator-= (const Student&, const Student&); 

Они определяются следующим образом:

Student operator+= (const Student& stu1, const Student& stu2) 
{ 
    if (stu1.getClassL() >= stu2.getClassL()) 
    { 
     for (int count = 0; count < stu.getClassL(); count++) 
      stu1.array_d[count] += stu2.array_d[count]; 

     return (stu1); 
    } 
    else if (stu1.getClassL() < stu2.getClassL()) 
    { 
     for (int count = 0; count < stu1.getClassL(); count++) 
       stu1.array_d[count] += stu2.array_d[count]; 

     return (stu1); 
    } 
} 

Student operator-= (const Student& stu1, const Student& stu2) 
{ 
    if (stu1.getClassL() >= stu2.getClassL()) 
    { 
     for (int count = 0; count < stu2.getClassL(); count++) 
      stu1.array_d[count] -= stu2.array_d[count]; 

     return (stu1); 
    } 
    else if (stu1.getClassL() < stu2.getClassL()) 
    { 
     for (int count = 0; count < stu1.getClassL(); count++) 
       stu1.array_d[count] -= stu2.array_d[count]; 

     return (stu1); 
    } 
} 

В основном то, что происходит здесь, я сравниваю два объекта с массивами, которые различаются по размеру на основе classL. функция getClassL() просто: int Student::getClassL() const {return classLists;}

В случае, если вам интересно, я перегружен большой тройки таким образом:

1. Destructor:Student::~Student() {delete [] array_d;}

2. Конструктор копирования:

Student::Student(const Student &student) 
{ 
    classLists = student.classLists; 
    array_d = student.array_d; 
} 

3. Оператор присваивания:

Student &Student::operator=(const Student &student) 
{ 
    classLists = student.classLists; 
    array_d = student.array_d; 
} 

Это странно, что + = работает, но - = не работает, так как они практически одинаковы. Я подозреваю, что моя проблема заключается в распределении моей динамической памяти, но я не уверен и ищу совет.

+3

'operator + =' и '- =' должен возвращать ссылки на текущий объект, а не на новый объект. Таким образом, ваш код, который принимает два объекта 'Student' для' + = 'и' - = ', не имеет особого смысла. Эти функции должны брать один объект «Студент» по ссылке и добавлять его в 'this' и возвращать' * this'. Теперь, если бы они были оператором '+' и '-', то две функции аргументов имели бы немного больше смысла. – PaulMcKenzie

+2

Неправильный конструктор копирования. Вам нужно выделить массив правильного размера и скопировать элементы. Ваше задание также неверно. См. Это: http: //stackoverflow.com/questions/4172722/what-is-the-rule-of-three – NathanOliver

+0

Поскольку вы копируете адрес массива в своем конструкторе копирования, возникает следующая проблема: пусть A, B - студенты, вы копируете B в A, B в какой-то момент уничтожается, а его массив удаляется в деструкторе, но A по-прежнему содержит указатель на эту ячейку памяти. Это можно решить, следуя советам @ NathanOliver. –

ответ

3

Совет, который вам даст, заключается в использовании std::vector и избавит вас от необходимости выполнять оператор присваивания, конструктор копирования и деструктор.

Но, сказав это, в коде есть несколько ошибок, не говоря уже о конструкторе копирования.

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

class Student 
{ 
    int num; 
    double *array_d; 

    public: 
     Student(const Student &student); 
     Student& operator=(const Student &student); 
     ~Student() { delete [] array_d; } 
     Student() : array_d(0), num(0) {} 
}; 

Конструктор копирования будет выглядеть примерно так:

Student::Student(const Student &student) : 
    array_d(new double[student.num]), num(student.num) 
{ 
    for (int i = 0; i < num; ++i) 
    array_d[i] = student.array_d[i]; 
} 

После того как вы это, оператор присваивания является тривиальным использованием copy/swap:

Student& Student::operator=(const Student &student) 
{ 
    Student temp = student; 
    std::swap(d_array, temp.d_array); 
    std::swap(num, temp.num); 
    return *this; 
} 

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


Следующая вещь, чтобы рассмотреть это вся ваша идея для оператора += и -=. Путь большинство программистов рассчитывают использовать это в этом контексте:

Student a; 
Student b; 
// assume a and b are initialized and have data... 
a += b; 

Если вы используете += или -= в иной форме, она становится неинтуитивными и просто странно. Таким образом, эти функции должны принимать один аргумент, а не два аргумента, и возвращать текущий объект (это текущий объект, который вы меняете).

Поэтому эти функции не должны быть другом функции, но функции-члены класса Student:

class Student 
{ 
    int num; 
    double *array_d; 

    public: 
     Student(const Student &student); 
     Student& operator=(const Student &student); 
     ~Student() { delete [] array_d; } 
     Student() : array_d(0), num(0) {} 
     Student& operator += (const Student& rhs); 
     Student& operator -= (const Student& rhs); 
}; 

Тогда реализация будет выглядеть примерно так для +=:

#include <algorithm> 
//... 
Student& operator+= (const Student& stu1) 
{ 
    int num_to_add = std::min(num, stu1.num); 
    for (int count = 0; count < num_to_add; ++count) 
     array_d[count] += stu1.array_d[count]; 
    return *this; 
} 

Аналогично, -= будет выглядеть как указано выше.Обратите внимание на использование std::min, чтобы определить, сколько добавить, вместо исходного кода с if/else.

+0

В качестве примечания, копирование и своп легко, но бесполезно медленно. Обычно это не имеет значения. –

1

Ваш вопрос больше посвящен вопросу о том, почему C++ 11/C++ 14 является лучшим C++, чем предыдущие, и почему его следует использовать как таковой.

Во-первых, как пояснил комментарий, операторы += и -= должны возвращать ссылку, а не копию. Если бы это был оператор функции-члена, он бы return *this, но в вашем случае это не так. Просто измените тип возврата на Student &.

Причина возникновения сбоя, также рассмотренная в комментариях, заключается в том, что при выполнении конструктора или присвоения копии право собственности на array_d принимается обоими объектами «Студент». Когда второй будет уничтожен, он попытается установить delete [] array_d, который был уже удален первым.

Оператор присваивания ничего не возвращает.Он должен вернуть * это;

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

Однако это ключевой момент, чтобы проиллюстрировать, почему STL так ценен. Если бы вы использовали std::vector<double>, оператор присваивания вектора скопировал бы все элементы для вас.

В двух операторских функциях (как +=, так и -=) предложение «else if» не имеет значения. Логически, если stu1.getClassL() не >= stu2.getClassL(), это может быть только <, поэтому сохраните время и удалите предложение else if, а также содержащие фигурные скобки.

Смежные вопросы