1

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

class AbstractProductPhoneNumber
{
protected:
    string countryCode;
    string phoneNumber;
public:
    AbstractProductPhoneNumber()
    {
    }
    AbstractProductPhoneNumber(string cc, string ph)
    {
        this->countryCode = cc;
        this->phoneNumber = ph;
    }
    AbstractProductPhoneNumber(AbstractProductPhoneNumber &pn)
    {
        cout << "AbstractProductPhoneNumber copy ctor\n";
        this->countryCode = pn.countryCode;
        this->phoneNumber = pn.phoneNumber;
    }
    AbstractProductPhoneNumber &operator = (AbstractProductPhoneNumber &pn)
    {
        if (this == &pn)
        {
            return *this;
        }
        cout << "AbstractProductPhoneNumber operator =\n";
        this->countryCode = pn.countryCode;
        this->phoneNumber = pn.phoneNumber;
        return *this;
    }
    void SetCountryCode(string cc)
    {
        this->countryCode = cc;
    }
    void SetPhoneNumber(string ph)
    {
        this->phoneNumber = ph;
    }
    virtual string GetPhoneNumber() = 0;//абстрактный метод должен вернуть номер телефона в том формате, который принят для конкретной страны
};

class AbstractProductAddress
{
protected:
    string Country;
    string City;
    string Street;
    string House;
    string Flat;
public:
    AbstractProductAddress()
    {
    }
    AbstractProductAddress(string cn, string ct, string st, string hm, string ft)
    {
        this->Country = cn;
        this->City = ct;
        this->Street = st;
        this->House = hm;
        this->Flat = ft;
    }
    void SetCountry(string cn)
    {
        this->Country = cn;
    }
    void SetCity(string ct)
    {
        this->City = ct;
    }
    void SetStreet(string st)
    {
        this->Street = st;
    }
    void SetHouse(string hm)
    {
        this->House = hm;
    }
    void SetFlat(string ft)
    {
        this->Flat = ft;
    }
    virtual string GetAddress() = 0;
};

class AbstractFactory
{
public:
    virtual AbstractProductPhoneNumber *CreatePhoneNumber() = 0;
    virtual AbstractProductAddress *CreateAddress() = 0;
};

class ConcreteProductPhoneNumberUA : public AbstractProductPhoneNumber
{
public:
    string GetPhoneNumber()
    {
        string str = "+";
        str += this->countryCode;
        str += " ";
        str += this->phoneNumber;
        return str;
    }
};
class ConcreteProductAddressUA : public AbstractProductAddress
{
public:
    string GetAddress()
    {
        string str = this->Country + ", city" + this->City + ", street" + this->Street + ", house" + this->House + ", flat" + this->Flat;
        return str;
    }
};
class ConcreteProductPhoneNumberUS : public AbstractProductPhoneNumber
{
public:
    string GetPhoneNumber()
    {
        string str = "+";
        str += this->countryCode;
        str += this->phoneNumber;
        return str;
    }
};
class ConcreteProductAddressUS : public AbstractProductAddress
{
public:
    string GetAddress()
    {
        string str = this->Flat;
        str += ", ";
        str += this->House;
        str += ", ";
        str += this->Street;
        str += ", ";
        str += this->City;
        str += ", ";
        str += this->Country;
        return str;
    }
};
class ConcreteFactoryUA : public AbstractFactory
{
    AbstractProductPhoneNumber *CreatePhoneNumber()
    {
        return new ConcreteProductPhoneNumberUA();
    }
    AbstractProductAddress *CreateAddress()
    {
        return new ConcreteProductAddressUA();
    }
};
class ConcreteFactoryUS : public AbstractFactory
{
    AbstractProductPhoneNumber *CreatePhoneNumber()
    {
        return new ConcreteProductPhoneNumberUS();
    }
    AbstractProductAddress *CreateAddress()
    {
        return new ConcreteProductAddressUS();
    }
};
class ContactItem//представляет элемент списка
{
private:
    AbstractProductPhoneNumber *pn;
    AbstractProductAddress *addr;
public:
    ContactItem()
    {
        this->pn = NULL;
        this->addr = NULL;
    }
    ~ContactItem()
    {
        delete this->pn;
        delete this->addr;
    }
    void CreateItem(AbstractFactory *af)
    {
        this->pn = af->CreatePhoneNumber();
        this->addr = af->CreateAddress();
        char A[100];
        cout << "Enter the phone number:\n";
        cin >> A;
        this->pn->SetPhoneNumber(A);
        cout << "Enter a country:\n";
        cin >> A;
        this->addr->SetCountry(A);
        cout << "Enter a city:\n";
        cin >> A;
        this->addr->SetCity(A);
        cout << "Enter a street:\n";
        cin >> A;
        this->addr->SetStreet(A);
        cout << "Enter a house:\n";
        cin >> A;
        this->addr->SetHouse(A);
        cout << "Enter a flat:\n";
        cin >> A;
        this->addr->SetFlat(A);
    }
    AbstractProductPhoneNumber *GetPn()
    {
        return this->pn;
    }
    AbstractProductAddress *GetAddr()
    {
        return this->addr;
    }
};
int main(int argc, char* argv[])
{
    vector<ContactItem> contactList;
    ConcreteFactoryUA ua;

    ContactItem one;
    one.CreateItem(&ua);
    contactList.push_back(one);

    ContactItem two;
    two.CreateItem(&ua);
    contactList.push_back(two);

    vector<ContactItem>::iterator p = contactList.begin();

    while (p != contactList.end())
    {
        //cout << (*p).GetPn()->GetPhoneNumber() << "\n";
        AbstractProductPhoneNumber *pn = (*p).GetPn();
        cout << pn->GetPhoneNumber() << "\n";

        //cout << (*p).GetAddr()->GetAddress() << "\n";
        AbstractProductAddress *addr = (*p).GetAddr();
        cout << addr->GetAddress() << "\n";
        p++;
    }

}
vp_arth
  • 27,179
  • А нельзя как-то подсократить код? И вообще, а сама ошибка где? – Qwertiy Jun 26 '19 at 17:18
  • В том то и дело, что ошибка возникает только при выполнении, сократить не могу, т.к. не знаю где именно возникает – Артем Мазуров Jun 26 '19 at 17:23
  • Не специалист, но если полностью очистить деструктор ContactItem всё починится) Т.е. на delete this->pn происходит segfault – vp_arth Jun 26 '19 at 17:36
  • Если ошибка при выполнении, то почему в заголовке вопроса написано "ошибка при компиляции"? Виртуальные деструкторы где? Выполняй по шагам и смотри, на какой строке возникнет ошибка. – Qwertiy Jun 26 '19 at 17:44

1 Answers1

1

Дело в том, что vector.push_back вызывает копирующий конструктор, копирует ваши элементы вместе с указателями.

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

Попробуйте хранить в векторе не объекты, а указатели на них, либо управляйте копирующим конструктором, добавив его явным образом.


Попробую продемонстрировать:

class A {
  public: int val = 0;
  A() {cout << "A()" << endl; }
  A(const A &a) {cout << "A(A): " << val << " = " << a.val << endl; }
  A(int val): val{val} {cout << "A("<<val<<")" << endl; }
  ~A() {cout << "~A()" << endl; }
};

int main(int argc, char* argv[])
{
    vector<A> aList;
    A a1(1), a2(2);
    aList.push_back(a1);
    aList.push_back(a2);
    cout << "loop: ";
    // for (auto a: aList) a.val; // Такой foreach будет копировать на каждой итерации 
    for (auto it = aList.begin(); it != aList.end(); ++it)
        cout << it->val << ", ";
    cout << "end" << endl;
}
A(1)
A(2)
A(A): 0 = 1
A(A): 0 = 2
A(A): 0 = 0
~A()
loop: 0, 0, end
~A()
~A()
~A()
~A()

Решение с хранением в векторе не объектов, а указателей:

int main(int argc, char* argv[])
{
    vector<A*> aList;
    A a1(1), a2(2);
    aList.push_back(&a1);
    aList.push_back(&a2);
    cout << "loop: ";
    for (auto a: aList)
        cout << a->val << ", ";
    cout << "end" << endl;
}
A(1)
A(2)
loop: 1, 2, end
~A()
~A()
vp_arth
  • 27,179
  • если вы имелли ввиду так сделать, то не работает, или я чего то недопонял ContactItem(const ContactItem & obj) { this->pn = obj.pn; this->addr = obj.addr; } – Артем Мазуров Jun 26 '19 at 18:14
  • Ну а толку вы те же указатели копируете? Это поведение по умолчанию. Вам нужно создавать копии объектов, а не указателей. Но весь этот путь не очень хороший, лучше избежать копирования объектов. Храните в векторе указатели на них. – vp_arth Jun 26 '19 at 18:17
  • Я добавил в ответ пример. Ну и не забывайте про то, что при использовании наследования деструкторы всегда должны быть виртуальными. – vp_arth Jun 26 '19 at 18:23
  • @vp_arth, если базовый класс не предназначен для полиморфного наследования, то он не должен иметь виртуальный деструктор, так что вы правы только в случаи полиморфного наследования – AR Hovsepyan Jun 26 '19 at 19:00
  • 1
    @ARHovsepyan, вы правы, не успел отредактировать комментарий – vp_arth Jun 26 '19 at 19:05
  • @vp_arth, кроме этого, если производные классы не имеют собственных членов, тогда, при полимофном наследовании тоже может не быть виртуальный деструктор – AR Hovsepyan Jun 26 '19 at 19:10
  • @ARHovsepyan Может, но не должен. Множество производных классов - открытое. Как вы опишете подобный контракт? – vp_arth Jun 26 '19 at 19:12
  • @vp_arth, в общем случаи никак, но для частной программы (или если иерархия является инструментом для разработки) можно, Очень частный случай, но все же тоже может иметь место... – AR Hovsepyan Jun 26 '19 at 19:29