Remove an element from the linked list

Question

How to remove an element from the linked list?

Is it correct way:

public E remove(E element) {
 Node search = currentNode;
 boolean nonStop = true;
 while(((search.previous) != null) && (nonStop)) {
 if(search.previous.toString().equals(element.toString())) {
 System.out.println("Item found !!!");
 search.previous = search.previous.previous;
 nonStop = false;
 } else {
 search = search.previous;
 }
 }
 currentNode = search;
 return null;
 }
public class Node<E> {
 public E data;
 public Node<E> previous;
 public Node(E data) {
 this.data = data;
 }
 public void printNode() {
 System.out.println("Node details: "+data);
 }
 @Override
 public String toString() {
 // TODO Auto-generated method stub
 return (String)data;
 }
}

The problem is when I am printing all elements, getAllElements() is NOT giving correct answer,is there any problem in remove() method or getAllElements

public void getAllElements() {
 Node<E> aNode = currentNode;
 while(aNode != null) {
 aNode.printNode();
 aNode = aNode.previous;
 }
 }

• johnchen902: What is the detail of your linked list? Is it single-linked or double-linked? Is it linear or circular?
• Thorbjørn Ravn Andersen: Any reason you are not using one if the standard Lists in the runtime?
• Ashish Aggarwal: In case you want to review your code or want to improve your code post your question on codereview.stackexchange.com
• JB Nizet: Why compare their toString() value instead of comparing them with equals()? Why reimplement a LinkedList when a standard one already exists?
• Viktor Seifert: One thing that is certainly bad is search.previous.toString().equals(element.toString()). You should use equals without using toString first.

Answer 1

The line

if(search.previous.toString().equals(element.toString())

calls to string on the node and not on the element.

Answer 2

It seems like your remove method does not really remove anything, you should update the pointers in your method so that nothing points toward the element you want to remove, and then garbage collector will the remove the element that nothing points to. Some pseudocode to illustrate what I mean:

public remove(Element element){
 for (Element e : myLinkedList){
 if (e.equals(element)){
 if (next != 0)
 previousPtr = nextPtr;
 else
 previousPtr = null;
 }
 }
}

Note this is not correct Java code, just pseudocode to give you an idea, I save some fun for you!! :)

Answer 3

Try this:

public void remove(E element)
{
 Node n = head; // This is the head of the linked list-- It is the starting node of your linked list: For your case "currentNode"
 Node tmp;
 while(n!=null && !n.data.equals(element))
 {
 tmp = n;
 n = n.previous;
 }
 if(n==null)
 {
 // Do your stuff
 System.out.println("Element "+element+" not found.");
 }
 else
 {
 // Do your stuff
 tmp.prev = n.prev;
 n.prev = null;
 System.out.println("Element "+element+" removed.");
 }
}
// Suggestion: This method name should be "printList()"
public void getAllElements()
{
 Node n = head; // In your case: "currentNode"
 while(n!=null)
 {
 n.printNode();
 n = n.previous;
 } 
}

Answer 4

Don't your elements have some kind of identifier? Then you can do it much simpler, like here: remove an object

Answer 5

public E remove(E element) {
 Node search = currentNode;
 boolean nonStop = true;
 while(((search.previous) != null) && (nonStop)) {
 if(search.previous.data.equals(element)) {
 System.out.println("Item found !!!");
 search.previous = search.previous.previous;
 nonStop = false;
 }
 search = search.previous;
 }
 return null;
 }

I have found the solution for that Issue, thanks everyone for yours kind support.