Return to Answer

##Comments

##Code Review

##Naming conventions

##Design

##Query

##Without double indirection

Comments

Code Review

Naming conventions

Design

Query

Without double indirection

class SinleLinkedList
{
    public:
        void remove(int value)
        {
            remove(head, value);
        }
    private:
        Node*     head;
        void remove(Node*& ptr, int value)
        {
            Node*   prev    = NULL     
            Node*   current = ptr;
    
            while(current && current->data != value)
            {
                prev    = current;
                current = prev->next;
            }
    
            // If current is not NULL then we fell out of the
            // above loop because the != failed so it must be equal
            if (current /*&& current->data == value*/ )
            {
                // Set the value we are going to update after delete.
                // If prev == NULL its the head, otherwise we update
                // next pointer from the prev element.
                Node*&  result = (prev == NULL) ? ptr : prev->next;
                
                // Save the next value.
                // Delete the old node and update.
                Node* next = current->next;
                delete current;
                result = next;
            }
        }
};
        

class SinleLinkedList
{
    public:
        void remove(int value)
        {
            remove(head, value);
        }
    private:
        Node*     head;
        void remove(Node*& ptr, int value)
        {
            Node*   prev    = NULL     
            Node*   current = ptr;
    
            while(current && current->data != value)
            {
                prev    = current;
                current = prev->next;
            }
    
            // If current is not NULL then we fell out of the
            // above loop because the != failed so it must be equal
            if (current /*&& current->data == value*/ )
            {
                // Set the value we are going to update after delete.
                // If prev == NULL its the head, otherwise we update
                // next pointer from the prev element.
                Node*&  result = (prev == nullptr) ? ptr : prev->next;
                
                // Save the next value.
                // Delete the old node and update.
                Node* next = current->next;
                delete current;
                result = next;
            }
        }
};
        

This is not really C++.
Its just some C code

In C++ it is common convention, that names of object (anything that can have its address taken) begins with a lower case letter (this includes function/method names). So I would rename to remove(). This is so it is easy to spot the names of user defined types (which begin with upper case letters).

By the way; why a singly linked list. That sounds like a premature optimization to me. Code is much simplified by a doubly linked list (with a sentential). Of course you are writing in C so you are probably trying to write optimal code down as close to the metal with no help from language semantics.

Double indirection is horrible. Its hard to read hard to use in the code. If you want to pass something that needs to be updated pass a reference to the object. Then it will be updated in the function.

A simple technique for traversing a singly linked list is to use two pointers. A pointer to the current element and a pointer to the previous element. Unlink-ing the element then becomes a simple assignment to the previous element.

This is not really C++.
It is just some C code.

In C++ it is common convention that names of objects (anything that can have its address taken) begins with a lower case letter (this includes function and method names). So I would rename Remove to remove. This is so it is easier to spot the names of user defined types (which should begin with an upper case letters).

By the way, why a singly linked list? That sounds like premature optimization to me. Code is much simpler using a doubly linked list (with a sentinel). Of course you are writing in C so you are probably trying to write optimal code down as close to the metal with no help from language semantics.

Double indirection is horrible. It is hard to read and hard to use in the code. If you want to pass something that needs to be updated pass a reference to the object. Then it will be updated in the function.

A simple technique for traversing a singly linked list is to use two pointers. A pointer to the current element and a pointer to the previous element. Unlinking the element then becomes a simple assignment to the previous element.