HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavaModerate

Given only a pointer to a node to be deleted in a singly linked list, how do you delete it?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
youdeletenodepointersinglydeletedonlyhowlistlinked

Problem

Example: If a linkedlist list contains 10->20->30->40, and 2nd node has to be deleted then the output should be 10->30->40

This question is attributed to Geeksforgeeks. Looking for code-review, optimizations and best-practices.

```
class Nodde {
Nodde next;
T item;

Nodde(T item) {
this.item = item;
}
}

class LinkedList {

private Nodde first;
private Nodde last;

public LinkedList(Nodde first) {
this.first = first;
}

@Override
public int hashCode() {
int hashCode = 1;
for (Nodde x = first; x != null; x = x.next)
hashCode = 31*hashCode + (x == null ? 0 : x.hashCode());
return hashCode;
}

@Override
public boolean equals(Object obj) {

if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
LinkedList other = (LinkedList) obj;
Nodde currentListNode = first;
Nodde otherListNode = other.first;

while (currentListNode != null && otherListNode != null) {

if (currentListNode.item != otherListNode.item) {
System.out.println(currentListNode.item);
return false;
}
currentListNode = currentListNode.next;
otherListNode = otherListNode.next;
}

return currentListNode == null && otherListNode == null;
}
}

public final class DeletePtr {

public static void delete(Nodde ptr) {

if (ptr == null) {
throw new IllegalArgumentException("The pointer cannot be null.");
}

if (ptr.next == null) {
throw new IllegalArgumentException("The last node cannot be deleted.");
}

p

Solution

I had tagged your question reinventing-the-wheel, because you're recreating a LinkedList, and Java already has one. @200_success has removed this tag for some reason, allowing me to post this answer:

Use Java's built-in LinkedList.

There's no reason for you to be writing your own implementation. Using Java's own LinkedList (full classified name: java.util.LinkedList), you can use removeAt(index) to remove a node. If you only have a reference to an object that is contained in the list, you can use remove(Object).

As it stands right now, you expose your nodes to the user of your class. You make THEM manage the LinkedList, whereas the LinkedList itself is nothing but a wrapper. If the user messes up and accidentally points the last node to another node, then makes a clone and compares them with equals or ==, the program will get into an infinite loop. If someone inserts a LinkedList which has the last node point to another node into something like a HashMap, the program will get into an infinite loop.

Searching for an element in the list requires the user to built their own implementation of a search function. In fact, doing anything with the list (other than removing an element) requires the user to get their hands dirty.

But lets say I wanted to use this LinkedList for a valid purpose.
I can't even remove the last node from the LinkedList. That could significantly limit my options.

Actually, removing itself is rather dirty because you've called it delete. This gives me certain assumptions - namely, that the object won't exist anymore after I have called the function.

I call delete(Node) and surprise surprise... the node still exists! Except it has different values now. This means you're altering the objects I'm holding in my hands. You're even altering the objects you're holding in your own hands - in the first and last fields.

There's another bug related to that: When I make a list of 2 Nodes, and I remove the first one, you end up having a LinkedList that contains 1 node... double! And if I were to add another node with a different item in it, last wouldn't even point to the proper node!

When you combine all these reasons, you should see that it is a bad idea to reinvent the wheel. Just use Java's LinkedList and the methods they provide. That way you don't have as much bugs, you don't force the user to make their hands dirty, you don't have to test whether your list works, and you give your user more freedom in how they can use the list.

Context

StackExchange Code Review Q#59349, answer score: 17

Revisions (0)

No revisions yet.