Clear function in a linked list node class C++

Question

I am having trouble with making sure that I have created a clear function for a linked list Node class. I am using delete this which I know can cause memory issues, but this is the only way that I can think of to assure that all of the objects in the linked list are deleted. The final lines of main() will still print out the value of the head node that should have been deleted. Is this an error in the method or is this due to the pointer still being associated with the object?

Clear method snippet

class Node {
private:
  Node *next = NULL;
  double value;
public:
  void clear();
};

void Node::clear() {
  cout << "Clear: " << this << ":" << value << endl;
  if(next != NULL){
    next -> clear();
  }
  delete this;
}

Full file

using namespace std;

class Node {
private:
  Node *next = NULL;
  double value;

public:
  Node(double);
  Node getNext(){return *next;} //inline
  void setNext(Node *newNext); //set *next
  double getValue(){return value;} //inline
  void setValue(double newValue) {value = newValue;} //inline
  void incValue(); //Increment value by the value of next node's value. If next is NULL do nothing.
  int sizeOf(); //return size of linked list
  double largest(); //return largest value in linked list
  double smallest(); //return smallest value in linked list
  double getSum(); //Get summation of all
  double average(); //return average of all values in the linked list
  void print(); //print all values in linked list
  void print_reverse(); //print all values in reverse order
  void clear(); //remove all nodes from linked list
};

Node::Node(double newValue) {
  value = newValue;
}
void Node::setNext(Node *newNext) {
  next = newNext;
}

void Node::incValue() {
  if(next != NULL) {
    double nextVal = next -> getValue();
    value += nextVal;
  }
}
int Node::sizeOf() {
  int count = 0;
  if(next != NULL)
    count = next -> sizeOf();

  count += 1;

  return count;
}
double Node::largest() {
  double large = value;
  if(next != NULL)
    large = next -> largest();

  if(value > large)
    large = value;

  return large;
}
double Node::smallest() {
  double small = value;
  if(next != NULL)
    small = next -> smallest();

  if(value < small)
    small = value;


  return small;

}
double Node::average() {
  double sum = getSum();
  int size = sizeOf();
  return sum/size;
}
double Node::getSum() {
  double sum = 0;
  int count = 0;
  if(next != NULL)
    sum += next -> getSum();
  sum += value;
  return sum;
}
void Node::print() {
  cout << value << endl;
  if(next != NULL)
    next -> print();
}
void Node::print_reverse() {
  if(next != NULL)
    next -> print_reverse();
  cout << value << endl;
}
void Node::clear() {
  cout << "Clear: " << this << ":" << value << endl;
  if(next != NULL){
    next -> clear();
  }
  delete this;
}




int main() {
  //set up linked list
  Node *head, *temp;
  temp = new Node(1);
  head = temp;
  temp = new Node(2);
  temp -> setNext(head);
  head = temp;
  temp = new Node(3);
  temp -> setNext(head);
  head = temp;
  temp = new Node(4);
  temp -> setNext(head);
  head = temp;
  temp = new Node(5);
  temp -> setNext(head);
  head = temp;
  temp = new Node(6);
  temp -> setNext(head);
  head = temp;
  temp = new Node(7);
  temp -> setNext(head);
  head = temp;
  temp = new Node(8);
  temp -> setNext(head);
  head = temp;
  //print
  cout << "Print\n";
  head -> print();
  //average
  cout << "Average\n";
  double av = head -> average();
  cout << av << endl;
  //print reverse
  cout << "Print reversed\n";
  head -> print_reverse();
  //smallest
  cout << "Smallest\n";
  double small = head -> smallest();
  cout << small << endl;
  //largest
  cout << "Largest\n";
  double  large = head -> largest();
  cout << large << endl;
  //size
  cout << "Size\n";
  int size = head -> sizeOf();
  cout << size << endl;
  //clear
  cout << "Clear\n";
  head -> clear();
  //clear print
  cout << "Clear print\n";
  head -> print();
  cout << "Clear size\n";
  cout << head -> sizeOf() << endl;

  //end of program
  cout << "End\n";
}

Answer 1

You should rarely (if ever) use delete this;. You also do not clear the next pointer so it becomes a dangling pointer. This memory likely still contains much of the data that was there, so when you traverse the list after "clearing" it, you're seeing the old data -- but note that this is only one of the many things that could happen, because accessing an object that has been destructed is undefined behavior.

Instead, consider doing this:

void Node::clear() {
  cout << "Clear: " << this << ":" << value << endl;
  if(next != NULL){
    next -> clear();
    delete next;
    next = NULL;
  }
}

Even better, do this in Node's destructor, and then you can just delete the target node:

void Node::~Node() {
  clear();
}

void Node::clear() {
  cout << "Clear: " << this << ":" << value << endl;
  if(next != NULL){
    delete next;
    next = NULL;
  }
}

Even better, make next a std::unique_ptr and then you can just reset it on clear(), destruction is automatic, and copying a Node is properly forbidden:

class Node {
private:
  std::unique_ptr<Node> next;
  double value;
public:
  void clear();
};

void Node::clear() {
  cout << "Clear: " << this << ":" << value << endl;
  next.reset(null);
}

Note that the last node (the head) cannot remove itself. As others have noted, clearing a node can't reasonably do anything except stop pointing at the next node. You need to have a separate class for a list, and clear that.

Answer 2

Everything after your clear() is undefined behavior as your variable head has been deleted in the clear function. You can just consider your print after that garbage.

As for a proper way of freeing everything, you should put your clear function outside of your Node struct:

void clear(Node* head)
{
  Node* next = head->next;
  delete head;
  if (next != nullptr)
    clear(next);
}

I think calling delete in the destructor is UB as delete calls the destructor for you. As mentionned in the comments, the delete is not present in the destructor, forget what I said!