我有一个问题,我的代码构建成功,但一旦我在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;
}
}
你的复制构造函数有问题。要想知道问题所在,最好能有你其他函数的文档。尤其是。
/**
* 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);
}
这比你现在要写和维护的代码要少很多。漏洞的地方也更少。