Return to Revisions

Comments

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

Code Review

Everything @matsjoyce said:

1. Don't need 'head'
2. Always use '{}' around sub block statements.

Naming conventions

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).

Design

Encapsulation is your friend.
A free function to delete elements from a list is the wrong way to go. You should have created a list class that contains nodes. Then add a member function to delete the node.

Query

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.

Without double indirection

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.

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;
            }
        }
};