Linked List Delete Node Function

Question

This code is supposed to delete every node from a linked list with a given value.

Are there any logical errors with this implementation? Did I do anything wrong or is there an area of this implementation that needs significant improvement. Any suggestions?

public boolean delete(int d) {
    Node tmpNode = head;
    Node prevNode = null;
    boolean deletedANode = false;

    if (head == null) {
        return deletedANode;
    }

    while (tmpNode != null) {
        if (tmpNode.data == d) {
            if (tmpNode == head) {
                head = head.next;
            }
            else {
                prevNode.next = tmpNode.next;
            }
         deletedANode = true;
         }

         prevNode = tmpNode;
         tmpNode = tmpNode.next;
    }

    return deletedANode;
}

Answer 1

Your code is buggy, if you have two Nodes with the same value in succession it will corrupt the list....

prevNode will be set to the deleted node tempNode, and if the next value also matches the input value you will be working with the wrong node as prevNode.

Also, why is d a good name for the input search value?

To avoid future bugs it is convenient to mark constant input values as final. It also can potentially help with performance

You need to change some code:

// added final, change input to `searchValue`
public boolean delete(final int searchValue) {
    Node tmpNode = head;
    Node prevNode = null;
    boolean deletedANode = false;

    /*
    This code is redundant, the while-loop does the same effective thing.
    if (head == null) {
        return deletedANode;
    }
    */

    while (tmpNode != null) {
        if (tmpNode.data == searchValue) {
            if (tmpNode == head) {
                head = head.next;
            } else { // fixed indenting/newline
                prevNode.next = tmpNode.next;
            }
            // fixed indenting
            deletedANode = true;
         } else {
             // only advance the prevNode when there's no match.
             prevNode = tmpNode;
         }
         tmpNode = tmpNode.next;
    }

    return deletedANode;
}