链接列表中的构造函数、析构函数和=操作符的问题。

问题描述 投票:0回答:1

我有一个问题,我的代码构建成功,但一旦我在Visual Studio上运行它,它就以 "Insufficient system resources exist to complete the requested service. "而停止。这段代码基本上是创建一个链接的列表,我实现了一个Set类来创建列表并对其进行基本操作。

我认为是我的copy构造函数、destructor和=operator的问题。以下是代码。

main.cpp

#include <iostream>
#include "Set.h"

using namespace std;

int  main()
{
    Set a;
    a.insert("aa");
    a.insert("bb");
    a.insert("cc");
    a.insert("dd");

    Set b;
    b.insert("ee");
    b.insert("ff");

    Set c;
    Set c(b);
    cout << "C is " << endl;
    c.dump();
    cout << endl;
    c = a;
    cout << "C is " << endl;
    c.dump();

Set.h

class Set
{
public:
    Set();
    bool empty() const;
    int size() const;
    bool insert(const ItemType& value);
    bool erase(const ItemType& value);
    bool contains(const ItemType& value) const;
    bool get(int pos, ItemType& value) const;
    void swap(Set& other);
    void dump();

    ~Set();
    Set(const Set& other);
    Set& operator=(Set other);

private:
    struct Node
    {
        ItemType data;
        struct Node* next;
        struct Node* prev;

    }m_dummy;

    Node *m_dummyPtr;
    int m_size;

};

Set.cpp /只复制构造函数、破坏函数和=操作符。

Set::~Set()
{
    Node* current = m_dummyPtr->next;
    Node* next;
    for (int i = 0; i < m_size;i++)
    {
        next = current->next;;
        delete current;
        current = next;
    }
    m_dummyPtr->next = nullptr;
    m_dummyPtr->prev = nullptr;

}

Set::Set(const Set& other)
{
    m_size = other.m_size;
    Node* p = other.m_dummyPtr->next;

    m_dummyPtr = &m_dummy;
    m_dummyPtr->data = {};
    m_dummyPtr->next = m_dummyPtr;
    m_dummyPtr->prev = m_dummyPtr;

    if (m_size == 0)
        ;
    else
    {
        for (int i = 0;i < m_size;i++)
        {
            insert(p->data);
            p = p->next;
        }
    }
}

Set& Set::operator=(Set other)
{
    if (this != &other) {
        swap(other);
    }
    return *this;
}

设置插入功能执行

bool Set::insert(const ItemType& value)
{
    if (m_size == 0)
    {
        Node* newNode = new Node;
        newNode->data = value;
        newNode->prev = &m_dummy;
        newNode->next = &m_dummy;

        m_dummy.next = newNode;
        m_dummy.prev = newNode;

        m_size++;
        return true;
    }
    else if (contains(value))
    {
        return false;
    }
    else
    {
        Node* newNode1 = new Node;
        Node* p = m_dummy.prev;
        m_dummy.prev = newNode1;
        newNode1->data = value;
        newNode1->next = &m_dummy;
        newNode1->prev = p;
        p->next = newNode1;
        m_size++;
        return true;
    }
}
c++ class constructor linked-list destructor
1个回答
0
投票

你的复制构造函数有问题。要想知道问题所在,最好能有你其他函数的文档。尤其是。

/**
 * Inserts `value` into `*this`, but only if `*this` does not already contain `value`.
 * Side effect: if `value` is inserted, the size is updated.
 */
bool Set::insert(const ItemType& value)

那个副作用是相关的!看看你的复制构造函数做了什么。它首先将大小设置为预期的 最后的 大小,然后它继续调用 insert() 为每个要插入的元素。每一次这样的调用都会增加 超出预期尺寸. 充其量,你是在看 m_size 是列表中节点数的两倍(直到复制副本,这时大小至少是它应该有的三倍,以此类推)。

然而,情况比这更糟。控制插入的循环将运行 m_size 次,而每次迭代都可能增加 m_size. 你的设置基本如下(我已经内嵌了以下内容 insert() 并删除了一些代码来集中解决这个问题)。)

for (int i = 0;i < m_size;i++)
    if (!contains(value))
        m_size++;

如果由于某些原因,你的 contains() 函数失败,总是返回 false你正在寻找一个无限循环,作为循环控制变量。i,以及结局条件。m_size,每次迭代都会递增。对于这个潜在的问题,一个快速的创可贴是循环直到 i < other.m_size,因为对方的尺寸不会改变。更好的办法是放弃 i 循环往复 p == &other.m_dummy (我不知道你为什么要把空间浪费在...) m_dummyPtr). 不过,这还是没有解决根本问题。


在编写代码时,你应该已经很清楚代码的作用;你应该能够用英语(或者你喜欢的人类语言)解释代码应该做什么。在本例中,看起来意图是让复制构造函数首先初始化 *this 作为一个空列表,然后将元素从 other*this. 所以要这么做。C++11给我们提供了一个叫做 委托构造函数 让这一过程无需重复代码,在这种情况下,可能会避免逻辑错误。

Set::Set(const Set& other) : Set()  // Delegate to construct an empty list
{
    // Copy the nodes of `other` to `*this`.
    Node* p = other.m_dummyPtr->next;
    if (other.m_size == 0)
        ;
    else
    {
        for (int i = 0;i < other.m_size;i++)
        {
            insert(p->data);
            p = p->next;
        }
    }
}

我再免费扔一个改进的方法。不要写额外的代码来处理那些本来可以自己处理的特殊情况。(这个建议也适用于 insert() 函数)。) 使用哨兵节点的一个好处是(该哨兵节点的 m_dummy 字段)是你经常不需要考虑一个空列表。然而你却为这种情况写了额外的代码。对于哨兵节点,如果你的主代码路径不能处理一个空列表,你很可能有一个逻辑错误。这里是你的 insert() 函数,而不检查是否为空。

Set::Set(const Set& other) : Set()
{
    // Copy the nodes of `other` to `*this`.
    Node* p = other.m_dummyPtr->next;
    for (int i = 0;i < other.m_size;i++)
    {
        insert(p->data);
        p = p->next;
    }
}

这仍然有效,因为当 other.m_size == 0所以没有必要为这个条件增加一个额外的检查。此外,我们还可以通过去掉掉一个叫 i:

Set::Set(const Set& other) : Set()
{
    // Copy the nodes of `other` to `*this`.
    for (Node* p = other.m_dummy.next; p != &other.m_dummy; p = p->next)
        insert(p->data);
}

这比你现在要写和维护的代码要少很多。漏洞的地方也更少。

© www.soinside.com 2019 - 2024. All rights reserved.