链表中的构造函数、析构函数和 = 运算符的问题

Question

所以我遇到了一个问题，我的代码构建成功，但是一旦我在 Visual Studio 上运行它，它就会停止并显示“系统资源不足，无法完成请求的服务。”。代码基本上是创建一个链表，我正在实现一个 Set 类来创建列表并对其进行基本操作。

我认为这是我的复制构造函数、析构函数和 = 运算符的问题。下面是代码：

ma​​in.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 //仅复制构造函数、析构函数和=operator

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;
    }
}

Answer 1

您的复制构造函数有问题。要查看问题，有其他功能的文档会有所帮助。特别是：

/**
 * 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)

那个副作用是相关的！看看你的复制构造函数做了什么。它首先将大小设置为预期的 final 大小，然后继续为每个要插入的元素调用 insert()。每个这样的调用都会增加大小超出预期的最终大小。充其量，您看到m_size 是列表中节点数量的两倍（直到副本被复制，当大小至少变为应有的三倍时，等等）。

然而，情况比这更糟。控制插入的循环将运行m_size 次，并且每次迭代都可能增加m_size。您的设置基本上如下（我已内联 insert() 并删除了一些代码以专注于该问题）：

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

如果由于某种原因，您的 contains() 函数总是返回 false 而失败，那么您正在查看一个无限循环，作为循环控制变量 i 和结束条件 @987654332 @, 将随着每次迭代而递增。这个潜在问题的快速创可贴是循环直到i < other.m_size，因为另一个的大小不会改变。更好的是删除i 并循环直到p == &other.m_dummy（我不知道你为什么在m_dummyPtr 上浪费空间）。不过，这并没有解决根本问题。

---

在编写代码时，您应该已经对代码的作用有一个很好的了解；您应该能够用英语（或您喜欢的人类语言）解释代码应该做什么。在这种情况下，复制构造函数的意图似乎是首先将*this 初始化为一个空列表，然后将元素从other 复制到*this。就这样做吧。 C++11 为我们提供了一个名为 delegating constructor 的工具，它可以在不重复代码的情况下完成此操作，并且在这种情况下可以防止逻辑错误。

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);
}

编写和维护的代码比您目前拥有的要少得多。更少的地方可以让 bug 潜入。