2

I am currently making a linked List program using Nodes(not that i know of any other way) and I have come upon a problem about creating a deep copy and getting rid of all my Nodes and Sentinels with my ~List(). Deleting the Nodes is not a problem, but the sentinels are since the first one is not assigned a index value.

List::~List()
{
  for(size_t i=0; i<size; i++)
    {
      _setCurrentIndex(i);
      if(current && curent->next == NULL)
    {
      Node *temp = current->next;
      delete temp;
      delete current;
    }
      else
    {
      Node *old = current;
      current = current->next;
      delete old;
    }
    }
}

List::List(const List & orig)
{
for(size_t i=0; i<size; i++)
{
 if(i==0)
  {
   Node *copyFront = new Node; //the first sentinel
   copyFront->data = orig.front->data; //front is defined in private in list.h
   copyFront->prev = NULL; // it is defined as a Node (same for rear)
  }
 else if(0<=i && i<size) //put in i<size b/c 0<=i would always be true
  {
   _setCurrentIndex(i) //sets what current is and currentIndex which pts to diff Nodes
   Node *copy = new Node;
   copy->data = current->data; 
   copy->next = current->next;
   current = current->next;
  }
 else if(i+1 == size)
  {
   Node *copyRear = new Node; //making the last sentinel, but it has to be
   copyRear->data = orig.rear->data; //after data Node
   copyRear->next = NULL;
  }
 }
}

I am seeking advice and comments on this code on how to proceed next or what to change if something is dreadfully wrong!

Kurt E
  • 367
  • 1
  • 2
  • 9
  • 2
    Looks *way* too complicated. A linked list should consist of nodes of the form `struct node { node * next; node * prev; T data; };` or something like that, and you can use `NULL` to indicate the end. – Kerrek SB Oct 18 '12 at 01:51
  • If you were using `std::unique_ptr` to own your nodes your destructor would be empty. – David Oct 18 '12 at 01:59
  • That is my struct for my nodes which one is too complicated? – Kurt E Oct 18 '12 at 01:59
  • No, your struct for the list is too complicated. – Synxis Oct 18 '12 at 08:44

1 Answers1

0

Linked lists are templates allowing any type of variable to sit in them. In my honest opinion, you'd be best off using the std::list which requires the #include <list> header file.

Of course, if you really want the experience of writing a linked list class yourself then the following code makes a deep copy of a list:

List::List( const List& other) {
    if( other.head_ != nullptr) {
        head_ = new Node( other.head_->item_);  // copy first node

        assert( head_ != nullptr);  // ensure that the memory was allocated correctly

        // copy the rest of the list
        Node* pnew = head_;
        // loop through the list until you reach the end (i.e. a node that's nullptr)
        for( Node* porig( other.head_->next_); porig != nullptr; porig = porig->next_) {
            // assign the next node in the destination list to the next node in the paramter's list
            pnew->next_ = new Node( porig->item_);
            assert( pnew->next_ != nullptr);  // ensure that the memory was allocated correctly
            pnew = pnew->next_;  // move onto the newly created node in the destination list
        }
    }
    else
        // if the parameter is empty then the destination list will be empty as well
        head_ = nullptr;
}

As for the destructor, you just need to run through the list deleting the nodes as you go:

List::~List() {
    while( head_ != nullptr) {  // keep looping until the list gets to the end
        // make a second pointer to the node you are about to delete (so you don't lose track of it)
        Node* pn( head_);
        // move the head_ onto the next node essentially "removing" the first node from your list
        head_ = head_->next_;
        // delete the node that you've just "removed" from your list
        delete pn;
    }
}

I've tried to make the comments clear up anything that might be unclear.

Edward
  • 235
  • 4
  • 18