首页
学习
活动
专区
圈层
工具
发布
首页
学习
活动
专区
圈层
工具
MCP广场
社区首页 >问答首页 >双链接列表实现[C++]

双链接列表实现[C++]
EN

Code Review用户
提问于 2018-03-06 07:09:01
回答 3查看 4.1K关注 0票数 4

C++报道。我正在使用两个单独的类实现双链接列表:一个类名为node,包含标准的双链接列表参数,另一个类称为dList,它包含更改双链接列表的函数。这是我的node课程:

代码语言:javascript
运行
复制
class node {
  public:
    node *prev;
    node *next;
    int key;
    char type;
};

这是我的dList课程:

代码语言:javascript
运行
复制
class dList {
  private:
    node *head; // dummy head
    node *tail; // dummy tail

  public:
    dList() { // default constructor, creates empty list
      cout << "in default constructor" << endl; // to debug
      head = nullptr;
      tail = nullptr;
    }
    // destructor
    ~dList() {
      cout << "in destructor" << endl; // to debug
      node *ptr = head;
      while (ptr != nullptr) {
        delete ptr;
        ptr = ptr->next;
      }
      tail = nullptr;
    }

    // searches list for occurence of int parameter and returns pointer to node containing key
    node *search(int k);

    // outputs all keys that have type equal to character parameter, front to back
    void find(char t);

    // parametrized constructor, initialize list w/ contents of arrays
    dList(int arrayNums[], char arrayChars[], int size);

    // creates new node at front of list
    void addFront(int k, char t);

    // creates new node at back of list
    void addBack(int k, char t);

    // moves node pointed to by parameter to front of list
    void moveFront(node* ptr);

    // moves node pointed to by parameter to back of list
    void moveBack(node* ptr);

    // outputs first int elements of list, starting at front or back depending on char parameter
    void out(int num, char = 'f');

    // splits list in two
    node *split(node* headRef);

    // merges two linked lists
    node *mergeLists(node* first, node* second);

    // performs mergeSort on linked list
    node *mergeSort(node* headRef);

    // peforms an O(NlogN) sort on list; list should be in increasing order based on integer key
    void sort();
};

除了sort()函数和与它相关的所有函数(split, mergeLists, mergeSort)之外,我已经计算出了所有的函数。我主要是寻找批评和改进。如果你们能够查看我的代码并帮助我优化它(压缩和删除不必要的行),以及帮助我确保正确地使用指针,这将是非常感谢的!

下面是我声明的函数(不包括任何与sort相关的函数):

代码语言:javascript
运行
复制
// parametrized constructor, initialize list w/ contents of arrays
dList::dList(int arrayNums[], char arrayChars[], int size) {
  node *newNode = nullptr;
  //cout << "parametrized constructor called" << endl;
  for (int i = 0; i < size; i++) {
    //cout << "in for loop" << endl;
    if (head == nullptr) {
      //cout << "in first if statement" << endl;
      newNode = new node;
      newNode->key = arrayNums[i];
      newNode->type = arrayChars[i];
      newNode->prev = nullptr;
      newNode->next = nullptr;
      head = newNode;
      tail = head;
    }

    else {
      //cout << "in else statement" << endl;
      newNode = new node;
      newNode->key = arrayNums[i];
      newNode->type = arrayChars[i];
      newNode->prev = tail;
      tail->next = newNode;
      tail = newNode;
    }
  }
}

void dList::addFront(int k, char t) { // creates new node at front of list
  node *newNode = new node;
  newNode->key = k;
  newNode->type = t;

  if (head != nullptr) {
    newNode->next = head;
    head->prev = newNode;
    head = newNode;
  }
  else {
    newNode->prev = nullptr;
    newNode->next = nullptr;
    head = newNode;
    tail = nullptr;
  }
}

void dList::addBack(int k, char t) { // creates new node at back of list
  node *newNode = new node;
  newNode->key = k;
  newNode->type = t;

  node *ptr = head;

  if (ptr != nullptr) {
    while (ptr->next != nullptr) {
      ptr = ptr->next;
    }
    ptr->next = newNode;
    newNode->next = nullptr;
    newNode->prev = ptr;
  }
  else {
    newNode->next = nullptr;
    newNode->prev = nullptr;
    ptr = newNode;
    tail = ptr;
  }
}

void dList::moveFront(node *ptr) { // moves node pointed to by parameter to front of list
  //node *tempHead = head;
  //node *tempTail = tail;

  if (ptr == head) {
    return;
  }
  else if (ptr == tail) {
    ptr->prev->next = nullptr;
    tail = ptr->prev;
  }
  else {
    ptr->prev->next = ptr->next;
    ptr->next->prev = ptr->prev;
  }

  head->prev = ptr;
  ptr->next = head;
  ptr->prev = nullptr;
  head = ptr;
}

void dList::moveBack(node *ptr) { // moves node pointed to by parameter to back of list
  //node *tempHead = head;
  //node *tempTail = tail;

  if (ptr == tail) {
    return;
  }
  else if (ptr == head) {
    head = ptr->next;
    tail->next = ptr;
    ptr->prev = tail;
    ptr->next = nullptr;
    tail = ptr;
  }
  else {
    ptr->prev->next = ptr->next;
    ptr->next->prev = ptr->prev;
    tail->next = ptr;
    ptr->prev = tail;
    tail = ptr;
  }
}
// searches list for occurence of int parameter and returns pointer to node containing key
node *dList::search(int k) {
  node *ptr = head;
  while (ptr->next != nullptr) {
    if (ptr->key == k) {
      return ptr;
    }
    else {
      ptr = ptr->next;
    }
  }

  return nullptr;
}
// outputs all keys that have type equal to character parameter, front to back
void dList::find(char t) {
  node *ptr = head;
  while (ptr->next != nullptr) {
    if (ptr->type == t) {
      cout << ptr->key << " " << ptr->type << "   ";
      ptr = ptr->next;
    }
    else {
      ptr = ptr->next;
    }
  }
}
// outputs first num elements of list, starting at front or back depending on char parameter
void dList::out(int num, char side) {
  cout << "out() called" << endl;
  if (side == 'b') {
    node *ptr = tail;
    for (int i = 0; i < num; i++) {
      cout << ptr->key << " " << ptr->type << "   ";
      ptr = ptr->prev;
    }
  }
  else {
    node *ptr = head;
    for (int i = 0; i < num; i++) {
      cout << ptr->key << " " << ptr->type << "   ";
      ptr = ptr->next;
    }
  }
}

下面是我用来测试的main.cpp文件:

代码语言:javascript
运行
复制
#include <iostream>
using namespace std;
#include "dList.cpp"
#define SMALL 200
#define MAX 100000
#define ROUNDS 100
int main(){
    int i, x[MAX];
    char ch[MAX];

    for(i=0;i<SMALL;i++) {
      x[i] = (2*SMALL)-i;
      ch[i] = 'a' + (i % 26);
    }
    dList A(x,ch,SMALL), B;
    A.out(10);
    node *tmp = A.search(2*SMALL-8);
    A.moveFront(tmp);
    A.out(10);
    A.moveBack(tmp);
    A.out(10);
    A.find('b');
    A.sort();
    A.out(10);
    A.out(10,'b');
    A.addBack(500,'d');
    A.addFront(501,'z');
    A.out(10);
    A.out(10,'b');
    B.addFront(1,'a');
    B.addBack(2,'b');
    B.out(2);

    for(int j=0; j<ROUNDS; j++){
      cout << endl << "round " << j << endl;
      for(i=0;i<MAX;i++) {x[i] = 2*MAX-i; ch[i] = 'a'+ (i%26);}
      dList A(x,ch,MAX);
      node *tmp = A.search(2*MAX-8);
      A.moveFront(tmp);
      A.moveBack(tmp);
      A.sort();
      A.out(10);
      A.out(10,'b');
    }
}

(作为一种奖励,如果你们能给我一些关于如何实现双链接列表合并的建议,我会很高兴的!)

谢谢你们!

EN

回答 3

Code Review用户

发布于 2018-03-06 17:15:17

审查工作只是一个简短的开始:

  1. 您违反了第三条规则/第五条规则: dList() ~dList(),您至少丢失了: dlist(const &) dlist& operator=(const &)标准提供的默认值严重错误。虽然假设至少是C++11,但您也希望: dlist( dlist& &) dlist& operator=(dlist&&)
  2. 以迭代方式删除dtor中的节点而不是重复删除节点的优点。但之后你又犯了免费使用后的罪行。通过在释放节点之前读取指向下一个节点的指针来修正此问题。而且,在拆毁房子之前粉刷墙壁是没有用的。

除了说它似乎是一个非常漏洞百出的抽象之外,我没有回顾您的接口和实现的其余部分。也许能解决这个问题?

票数 2
EN

Code Review用户

发布于 2018-03-06 17:50:58

设计

我会将Node作为一个私有类放在dList中。这是一个实现细节,除了你的班级,没人需要知道。

我会查一下如何使用哨兵对象。哨兵是列表中的一个假对象,它确实简化了代码,因为您从来不需要处理头/尾为空的问题。看看这个:https://codereview.stackexchange.com/a/126007/507

对于列表来说,接口并不是很标准的。您可能需要查看标准容器的设计,以了解标准方法是什么。

命名惯例。对于用户定义的类型,使用大写首字母更传统。然后用小写字母表示函数和对象。

代码语言:javascript
运行
复制
  Node(12, 'k');       // Uppercase letter therefore type.
                       // This is creating an object.

  node(12, 'k');       // lowercase letter therefore function
                       // This is calling a function

代码评审

三的

规则您未能实现3(或5)的规则。 { dList x; x.addBack(12, 'g'); dList y(x); // Woops shallow copy of x made // Even though you did not define the copy constructor } // BOOM. Code blows up because of double delete on the same pointer. 如果您定义了复制构造函数/赋值运算符或析构函数,那么您可能应该定义所有这三个。

通过已删除指针

访问

代码语言:javascript
运行
复制
  while (ptr != nullptr) {
    delete ptr;
    ptr = ptr->next;    // You just deleted ptr
                        // This is no longer yours to access and doing
                        // so is UB (the whole page may have been released
                        // back to the OS resulting in a page fault.


                        // Looking from a real world perspective the delete
                        // adds this block back to the memory management
                        // library and could easily overwrite parts of the 
                        // block in the processes. So what you read here
                        // is potentially garbage.
  }

干燥您的代码

在if语句的两边都会重复这些代码。为什么不删除重复的代码,这样它就会先发生呢?

代码语言:javascript
运行
复制
if (head == nullptr) {
  //cout << "in first if statement" << endl;
  newNode = new node;
  newNode->key = arrayNums[i];
  newNode->type = arrayChars[i];
  newNode->prev = nullptr;
  newNode->next = nullptr;
  head = newNode;
  tail = head;
}

else {
  //cout << "in else statement" << endl;
  newNode = new node;
  newNode->key = arrayNums[i];
  newNode->type = arrayChars[i];
  newNode->prev = tail;
  tail->next = newNode;
  tail = newNode;
}

简化了代码.

代码语言:javascript
运行
复制
  node *newNode = new node;
  newNode->key = k;
  newNode->type = t;

更容易写成:

代码语言:javascript
运行
复制
    node* newNode = new node {nullptr, nullptr, k, t};

打印

在C++中打印的正常方法是使用operator<<

代码语言:javascript
运行
复制
std::cout << 5 << "\n";

您可以通过定义这个运算符为您的类实现此操作。

代码语言:javascript
运行
复制
std::ostream& operator<<(std::ostream& s, dlist const& object)
{
    object.print(s);  // simplest way to ask object to print itself
                      // to the stream (pass the stream).
    return s;
}
票数 2
EN

Code Review用户

发布于 2018-03-07 10:21:38

我同意其他人的说法,但以下是一些补充意见。

  1. 在可能的情况下使用默认初始化:类节点{ public:节点*prev = nullptr;节点*next = nullptr;int键;char类型;};
  2. 为节点类提供一个构造函数,该构造函数传递密钥和类型类节点{ public: node(int Key,char Type):key( key ),type( type ) {} node *prev = nullptr;节点*next = nullptr;int key;char type;};让我们看看这是如何影响您的助手函数addFront首先无效dList::addFront(int,char ){ //在列表节点前面创建新节点*newNode =新节点( k,t);newNode->next = head;if (head != nullptr) { head->prev = newNode;} newNode;//如果您的列表是用空数组构造的,那么如果(tail == nullptr) { tail = newNode;}当您有指向尾的指针时,您是否有理由在addBack中遍历该列表?void::addBack(int,char ){ //在列表//前面创建新节点如果没有head创建addFront的快捷方式If (head == nullptr) { addFront(k,t);}节点*newNode =新节点( k,t);尾->next= newNode newNode->prev ===;newNode;}
  3. 更多的是个人偏好,但我宁愿使用列表中的私有结构,也不愿使用完全不透明的类,但无论如何。
  4. 您的问题是标记为C++,但是在构造函数dList(int arrayNums[], char arrayChars[], int size);中使用普通的C数组。在这里,std::vector会更好。还想一想,当其中一个数组比大小更短时会发生什么?dList::dList(const std::vector& arrayNums,const std::vector& arrayChars) { if (arrayNums.size() = arrayChars.size()) {//在这里做一些事情,例如抛出异常或任何您想要的} (std::size_t i= 0;i< arrayNums.size();++i) { addBack(arrayNums我,arrayChars我);}}
票数 1
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/188937

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档