0

I am trying to remove a node from a linked list, when given the value of the node to remove. Here is my code:

    virtual void remove(T value) {
    cout << "In remove" << endl;
    if(value == my_list) {
        Node *current = my_list;
        my_list = my_list->next;
        delete current;
    }
    else {
        Node *ptr = my_list;
        while (ptr != NULL) {
            if(ptr == value) {
                Node *current = ptr->next;
                ptr->next = ptr->next->next;
                delete current;
                num_items--;
            }
            else{
                ptr = ptr->next;
            }
        }
    }
    
};

I keep receiving the error:

LinkedList.h:106:15: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
  if(value == my_list) {
     ~~~~~~^~~~~~~~~~
LinkedList.h:114:21: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
          if(ptr == value) {

I think my problem is "value" is the value of the node and not the position of the node, but I'm not sure how to change that.

EDITED: Here is the declaration of all the variables:

template <class T>
class LinkedList: public LinkedListInterface<T>
{
private:
struct Node {
    T data;
    Node* next;
    Node(const T& the_data, Node* next_val = NULL) :
    data(the_data) {next = next_val;}
};

Node *my_list;
int num_items;
public:

LinkedList(void) {
    my_list = NULL;
    num_items = 0;
    cout << "In constructor" << endl;
};
virtual ~LinkedList(void) {
    cout << "In deconstructor" << endl;
    while(my_list != NULL) {
        Node *current = my_list;
        my_list = my_list->next;
        delete current;
    };
};
sabyu_2020
  • 1
  • 1
  • [Handy reading](https://stackoverflow.com/a/22122095/4581301). Pay special attention to pointer-to-pointer trick used in the community addition. That trick can be redeployed to make most of the major operations in a singly linked list much, much easier. – user4581301 Oct 31 '20 at 21:39
  • Probably should make why I brought that up clear. You could be doing some unexpected voodoo behind the scenes in `operator==`, but it looks a lot like `if(ptr == value)` finds a node and then `Node *current = ptr->next;` queues up the NEXT node for removal and destruction. The link above solves all that messiness. – user4581301 Oct 31 '20 at 21:44

2 Answers2

0

I suppose something like that (but it's difficult because your code is not complete : what is the accessor for value exactly ?) :

virtual void remove(T value) {
    cout << "In remove" << endl;
    if(value == my_list->data) {
        Node *current = my_list;
        my_list = my_list->next;
        delete current;
    }
    else {
        Node *ptr = my_list;
        while (ptr != NULL) {
            if(ptr->data == value) {
                Node *current = ptr->next;
                ptr->next = ptr->next->next;
                delete current;
                num_items--;
            }
            else{
                ptr = ptr->next;
            }
        }
    }
    
};
dspr
  • 2,383
  • 2
  • 15
  • 19
0

unfortunately I don't have full insight of what "Node" contains.

If node contains "value" field then your code should look like this:

virtual void remove(T value) {
    cout << "In remove" << endl;
    // my_list replaced with my_list -> value
    if(value == my_list->value) {
        Node *current = my_list;
        my_list = my_list->next;
        delete current;
    }
    else {
        Node *ptr = my_list;
        while (ptr != NULL) {
             // ptr replaced with ptr-> value
            if(ptr->value == value) {
                Node *current = ptr->next;
                ptr->next = ptr->next->next;
                delete current;
                num_items--;
            }
            else{
                ptr = ptr->next;
            }
        }
    }
    
};

What you did wrong is, you compared address of node element with integer value.