проблема перегрузки оператора присваивания

Question

Этот вопрос меня смутил.Первый фрагмент кода работает без сбоев, он отлично назначает s1-s2.Но вторая группа кода вызывает сбой программы.

Кто-нибудь имеет представление о том, почему это происходит или в чем проблема?

Код 1: (работает)

    s1.add(10, 30, 25, "Test Screen", false);
s1.add(13, 10, 5, "Name:XXX", false);
s1.add(13, 18, 30);
s1.remove(-1);
Screen s2 = s1;

Код 2: (сбой при назначении)

    Screen s1;

    if (1 != s1.add(10, 30, 25, "Test Screen", false))
        message("first add() has a problem");
   else if (2 != s1.add(13, 10, 5, "Name:XXX", false))
        message("second add() has a problem");
    else if (3 != s1.add(13, 18, 30))
        message("third add() has a problem");
    else if (3 != s1.remove(-1))
       message("first remove() has a problem");
    else {
        Screen s2 = s1;
}

Оператор назначения для класса экрана:

        Screen& operator=(const Screen &scr) {
        if (this != &scr){
            for (int i = 0; i < 50; i++) {
                if  (fields[i])
                    delete fields[i];
                fields[i] = new LField();
            }

            for (int i = 0; i < scr.numOfFields; i++)
                fields[i] = scr.fields[i];

            numOfFields = scr.numOfFields;
            currentField = scr.currentField;
        }
        return *this;
    }

Оператор назначения для класса поля:

LField& operator=(const LField &lfieldobj) {
        if (this != &lfieldobj) {

            if (lfieldobj.val) {
                if (val)
                    delete[] val;
                val = new char[strlen(lfieldobj.val) + 1];
                strcpy(val, lfieldobj.val);
            }
            else{
                //val = new char[1];
                val = "";
            }
            rowNum = lfieldobj.rowNum;
            colNum = lfieldobj.colNum;
            width = lfieldobj.width;
            canEdit = lfieldobj.canEdit;
            index = lfieldobj.index;

        }
        return *this;
    }

Любой вклад будет принята с благодарностью:)

Answer 1

        for (int i = 0; i < scr.numOfFields; i++)
            fields[i] = scr.fields[i];

Это не нормально, вы копируете указатель вместо указанного значения.Требуется глубокая копия.

Answer 2

Избавьтесь от текущего val и замените его на std::string.Избавьтесь от вашего fields и замените его на std::vector.Это должно позволить вам исключить обоих ваших перегруженных операторов присваивания;компилятор предоставит те, которые работают.Я предполагаю, что вы устраните проблемы с управлением памятью вместе с кодом.

В нынешнем виде, даже если вы «исправите» известные вам проблемы управления памятью, вы собираетесьбыть оставленным с тем фактом, что ваш код совершенно небезопасен перед лицом исключений (и использует new, поэтому он также не может избежать исключений).

Answer 3

Строка

Screen s2 = s1;

фактически вызывает конструктор копирования Screen, а не перегрузку оператора присваивания.

Например:

#include <iostream>
using namespace std;

class Screen
{
public:
        Screen() { }

        Screen(const Screen& s)
        {
                cout << "in `Screen::Screen(const Screen&)`" << endl;
        }

        Screen& operator=(const Screen& s)
        {
                cout << "in `Screen::operator=(const Screen&)`" << endl;
                return *this;
        }
};

int main()
{
        Screen s1;
        Screen s2 = s1;
}

print:

in Screen::Screen(const Screen&)

Я предполагаю, что ваш Screen конструктор копирования определен аналогично Screen::operator=(const Screen&), поэтому может потребоваться исправление для перегрузки оператора присваиваниятакже применяется к определению конструктора копирования.

Кроме того, как определяется fields член Screen?Если это похоже на

LField* fields[50];

, то внутри конструкторов вы должны инициализировать все LField* объекты в массиве как NULL, так как они имеют неопределенные начальные значения:

std::fill_n(fields, 50, static_cast<LField*>(NULL));

Без этой инициализации тест if (fields[i]) может успешно завершиться для некоторых i, даже если fields[i] не указывает на выделение, в результате чего ваша программа пытается delete указатель (и), которые не были возвращены new.

Answer 4

Что такое декларация члена поля?

LField* fields[50]?

Если это так, кто инициализирует левый член поля объекта в NULL?Я бы сказал, что никто ... оператор присваивания подобен конструктору копирования в C ++, и вы вызываете delete для недопустимого указателя.

Answer 5

Для сложных объектов лучше использовать copy и swap idum.
Это дает вам оператор присваивания с сильной гарантией исключения (транзакция безопасна). Но это также означает, что вам нужно рассмотреть сложное создание объекта только в одном месте (конструкторы).

Screen& Screen::operator=(Screen const& rhs)
{
    Screen tmp(rhs);
    this->swap(tmp);
    return *this;
}

void Screen::swap(Screen const& rhs) throw ()
{
     // Swap each of the members for this with rhs.
     // Use the same pattern for Field.
}

Answer 6

Мне удалось это исправить. В конце концов, это была проблема с выделением памяти:)