Smart pointers in doubly linked list

I'm trying to implement smart pointers in doubly linked list (university task). Before that I've done the same task in pure C with raw pointers. The problem is when I add new Node to List by addNode() more than twice I have crash (I use CodeLite(g++), Windows 10 64bit). I assume that the problem is my memory management (destructors of Node and List). But I feel I don't have enough qualification to solve that. So any help will be appreciated.

我正在尝试在双向链表(大学任务)中实现智能指针。在此之前,我使用原始指针在纯C中完成了相同的任务。问题是,当我通过addNode()添加新节点两次以上时,我崩溃了(我使用CodeLite(g ++),Windows 10 64位)。我假设问题是我的内存管理(Node和List的析构函数)。但我觉得我没有足够的资格来解决这个问题。所以任何帮助将不胜感激。

#include <stdlib.h>
#include <iostream>
#include <memory>

using namespace std;

template <typename T>
struct Node       {

    T Value;
    weak_ptr<Node<T>> prev;
    shared_ptr<Node<T>> next;

    ~Node() {
        while (next){
            prev = next->next;
            next.reset();
            next = prev.lock();
        }
    }

 };


template<typename T>
class List 
{public:  

     shared_ptr<Node<T>> start;
     weak_ptr<Node<T>> end;
     int size;

 public:
     List(): size(0){};
     ~List();

     void addNode (T value);

 };


template <typename T> List<T>::~List() 
 {   
     cout<<"List destructor"<<endl;
     while (start){
         auto sp = end.lock();
         end = start->next;
         start.reset(sp.get());
     }
 }

template <typename T> void List<T>::addNode(T value){
    Node<T>* nd = new Node<T>;

    if (size==0){
        start.reset(nd);
        start->Value = value;
        end = start;

    }

    else{
        auto sp = end.lock();
        auto sp2 = end.lock();
        sp->next.reset(nd);
        sp.reset(sp->next.get());
        sp->prev = sp2;
        sp->Value = value;
        cout<<"Value size is "<<size<<" "<<sp->Value<<endl;
        end = sp;

    }

    size++;
    return;
}




int main ()
{
 system("CLS");

  string a;
  string b;
  string c;

  a = "1 test";
  b = "2 test";
  c = "3 test";


  List<string> ls;
  ls.addNode(a);
  ls.addNode(b);
  ls.addNode(c);

  return 0;

}

1 个解决方案

#1

The point of using smart pointers is automatic memory management, so your List and Node destructors not only badly implemented but also redundant. Just remove them (or make them empty). Your only task can be to implement addNode() properly:

使用智能指针的关键是自动内存管理,因此您的List和Node析构函数不仅实现严重,而且冗余。只需删除它们(或将它们清空)。您唯一的任务可能是正确实现addNode():

template <typename T> void List<T>::addNode(T value){
    auto nd = std::make_shared<Node<T>>();
    nd->Value = value; // this should be done in Node constructor

    if (size++ == 0){
        start = nd;
    } else{
        auto sp = end.lock();
        nd->prev = sp;
        sp->next = nd;
    }
    end = nd;
}

When you deal with smart pointers normally you never call reset() and get() methods, only when you need to deal with raw pointers for various reasons (which is not the case here).

当你处理智能指针时,你通常不会调用reset()和get()方法,只有当你需要处理原始指针时出于各种原因(这不是这里的情况)。

Note: as stated in the comment though empty destructors would work correctly there could be issue with long list due to recursive call so you may want to implement only List destructor to avoid that. Again this should be done without using reset() and get(), just working with smart pointers normally:

注意:如注释中所述尽管空的析构函数可以正常工作,但由于递归调用可能会出现长列表问题,因此您可能只想实现List析构函数以避免这种情况。同样,这应该在不使用reset()和get()的情况下完成,只需使用智能指针:

~List()
{
    while( start )
       start = start->next;
}