Hackerrank delete a node I have been learning c++ for a few weeks now and I attempted this problem on Hackerrank Delete a node. I am aware of a similar post Delete a linked list node but the code seems more of a C structure than a C++ structure. Please feel free to suggest areas of Improvement and shorter way of writing this if any exists. Here is excerpt of the question


  You’re given the pointer to the head node of a linked list and the
  position of a node to delete. Delete the node at the given position
  and return the head node. A position of 0 indicates head, a position
  of 1 indicates one node away from the head and so on. The list may
  become empty after you delete the node.
  
  Input Format  You have to complete the Node* Delete(Node* head, int position) method which takes two arguments - the head of the
  linked list and the position of the node to delete. You should NOT
  read any input from stdin/console. The position will always be at least 0
  and less than the number of the elements in the list.
  
  Output Format  Delete the node at the given position and return the head of the updated linked list. Do NOT print anything to
  stdout/console.
  
  Sample Input
  
  1 --> 2 --> 3 --> NULL, position = 0  1 --> NULL , position = 0
  
  Sample Output
  
  2 --> 3 --> NULL NULL


/*
  Delete Node at a given position in a linked list 
  Node is defined as 
  struct Node
  {
     int data;
     struct Node *next;
  }
*/
Node* Delete(Node *head, int position)
{
  // Complete this method
    Node *temp = head;
    if(position == 0){
        head = head-&gt;next;        
        return head;
    }
    Node *temp2 = head;
    for(int i = 0 ; i &lt; position-1; i++){
        temp2 = temp2-&gt;next;              
    }
      temp = temp2-&gt;next-&gt;next;
      temp2-&gt;next = temp;    
    return head;
}
A few comments:


It is cleaner to define the position parameter as unsigned or
size_t as it cannot be negative. Same applies to the for loop counter.
There is a memory leak, as you don't do delete on the removed
node.
Names like temp and temp2 are not reader-friendly. How about
current_node? Speaking of which, the assignment to temp2 is redundant.
The code should validate that we don't go beyond the end of the list. We can of course assume that invalid index cannot be passed to the function, but it's a dangerous assumption that does not lead to defensive code.
While we're on the parameter validation topic; if we get passed an empty list (i.e. head == NULL), there will again be undefined behaviour irrespective of the value of position.
If we delete the last node, the code will behave in an undefined manner (most likely crash). Consider that temp2-&gt;next will be NULL in that case, which means that temp2-&gt;next-&gt;next won't be a happy statement.
Speaking of last comment, do you have unit tests in place? If so, it'd be good to have edge cases covered (delete first node, delete last node, empty list etc.)


Hope this helps.