patternjavaMinor
Double Linked List remove method
Viewed 0 times
methodremovedoublelistlinked
Problem
I am implementing some basic data structures to freshen up my memory and I wanted my
My implementation holds both the
DoubleLinkedList remove method to be reviewed. I feel like it has extra checks but I could not think of another way to implement.My implementation holds both the
head and the tail of the list making things a bit more complex.remove method:public void remove(T data) {
if (isEmpty()) {
throw new NoSuchElementException();
}
for (Node current = this.head; current != null; current = current.getNext()) {
if (current.getData().equals(data)) {
Node previous = current.getPrevious();
Node next = current.getNext();
if (this.size == 1) { // At head with size == 1
this.head = this.tail = null;
} else if (previous == null) { // At head with size > 1
this.head = next;
next.setPrevious(previous);
} else if (next == null) { // At tail
previous.setNext(next);
this.tail = previous;
} else { // Rest of cases
previous.setNext(next);
next.setPrevious(previous);
}
this.size--;
return;
}
}
throw new NoSuchElementException();
}Solution
I have couple suggestions:
Separate Logic
I would separate lookup and modify logic. Add a function to remove element and call it from this one leaving in this one only logic to lookup an item.
Simplify Checks
I think this should work and cover all your cases, no?
Separate Logic
I would separate lookup and modify logic. Add a function to remove element and call it from this one leaving in this one only logic to lookup an item.
public void remove(Node data) {
if (!data) {
throw new NoSuchElementException();
}
// Removing logic here
...
}
public void remove(T data) {
return remove(search(data));
}
public Node search(T data) {
for (Node current = this.head; current != null; current = current.getNext()) {
if (current.getData().equals(data)) {
return current;
}
}
return null;
}Simplify Checks
I think this should work and cover all your cases, no?
// If this is head of the list
if (previous == null) {
this.head = next;
}
else {
previous.setNext(next);
}
// If this is tail of the list
if (next == null) {
this.tail = previous;
}
else {
next.setPrevious(previous);
}Code Snippets
public void remove(Node<T> data) {
if (!data) {
throw new NoSuchElementException();
}
// Removing logic here
...
}
public void remove(T data) {
return remove(search(data));
}
public Node<T> search(T data) {
for (Node<T> current = this.head; current != null; current = current.getNext()) {
if (current.getData().equals(data)) {
return current;
}
}
return null;
}// If this is head of the list
if (previous == null) {
this.head = next;
}
else {
previous.setNext(next);
}
// If this is tail of the list
if (next == null) {
this.tail = previous;
}
else {
next.setPrevious(previous);
}Context
StackExchange Code Review Q#66413, answer score: 5
Revisions (0)
No revisions yet.