patternjavaModerate
Linked List Delete Node Function
Viewed 0 times
deletenodefunctionlistlinked
Problem
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?
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;
}Solution
Your code is buggy, if you have two Nodes with the same value in succession it will corrupt the list....
Also, why is
To avoid future bugs it is convenient to mark constant input values as
You need to change some code:
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 performanceYou 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;
}Code Snippets
// 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;
}Context
StackExchange Code Review Q#37099, answer score: 14
Revisions (0)
No revisions yet.