Remove all occurrences of item from a Linked List

Question

I've been working on this lab assignment for a few hours and can't understand why this code is not working. The question is to add the method int removeEvery(T item) that removes all occurrences of item and returns the number of removed items to a link list class that implements a link list interface.

This is my code: It removes some occurrences of the item, but not all of them.

public int removeEvery(T item){
int index = 0;
Node currentNode = firstNode;
for(int i = 1; i <= numberOfEntries; i++)
    {
    System.out.println(currentNode.getData());
        if (item.equals(currentNode.getData())){
            index++;
            remove(i);}
        else{
            currentNode = currentNode.getNextNode();}
    } 
        if(index != 0)
        return index;
    return -1;
}

Here is the remove method that was included in the LinkList class:

public T remove(int givenPosition)
{
  T result = null;                 // return value

  if ((givenPosition >= 1) && (givenPosition <= numberOfEntries))
  {
     assert !isEmpty();
     if (givenPosition == 1)        // case 1: remove first entry
     {
        result = firstNode.getData();     // save entry to be removed 
        firstNode = firstNode.getNextNode();
        if (numberOfEntries == 1)
           lastNode = null; // solitary entry was removed
        }
        else                           // case 2: givenPosition > 1
        {
           Node nodeBefore = getNodeAt(givenPosition - 1);
           Node nodeToRemove = nodeBefore.getNextNode();
           Node nodeAfter = nodeToRemove.getNextNode();
           nodeBefore.setNextNode(nodeAfter);  // disconnect the node to be removed
           result = nodeToRemove.getData();  // save entry to be removed

           if (givenPosition == numberOfEntries)
              lastNode = nodeBefore; // last node was removed
     } // end if

     numberOfEntries--;
  } // end if

  return result;                   // return removed entry, or 
                                  // null if operation fails
} // end remove

Answer 1

Here is a Java Code to delete all occurrences of an item from a linked list :

public class LinkedList{
    Node head;
    class Node{
        int data;
        Node next;
        Node(int d){data =d; next = null;}
    }

    public void push(int new_data){
        Node new_node = new Node(new_data);
        new_node.next = head;
        head = new_node;
    }

    public void insertAfter(Node givenNode, int new_data){
        if(givenNode == null)
            System.out.println("Given node cannot be empty");

        Node new_node = new Node(new_data);
        new_node.next = givenNode.next;
        givenNode.next = new_node;
    }

    public void append(int new_data){
        Node new_node = new Node(new_data);
        if(head == null)
            head = new_node;

        else{
        Node last = head;

        while(last.next != null)
            last = last.next;

        last.next = new_node;
    }
    }

    public void printList(){
        Node temp = head;

        while(temp != null){
            System.out.println(temp.data + " ");
            temp = temp.next;
        }
    }

    void deleteNode(int key){
        // Store head node
        Node temp = head, prev=null;
        // If head node itself holds the key or multiple occurrences of key
        while(temp != null && temp.data == key){
            head = temp.next;
            temp = head;
        }
        // Delete occurrences other than head
        while(temp != null){
            // Search for the key to be deleted, keep track of the
            // previous node as we need to change 'prev.next'
            while(temp != null && temp.data != key){
                prev = temp;
                temp = temp.next;
            }
            // If key was not present in linked list
            if(temp == null) return;
            // Unlink the node from linked list
            prev.next = temp.next;
            //Update Temp for next iteration of outer loop
            temp = prev.next;
        }
    }

     public static void main(String[] args){
         LinkedList llist = new LinkedList();

         llist.push(6);
         llist.append(7);
         llist.append(7);
         llist.append(7);
         llist.append(9);
         llist.push(10);
         llist.deleteNode(7);
         llist.printList();
     }
}

Output :

10 6 9

Answer 2

After removing a node, as @Vakimshaar suggested, you need to decrement the i because the node at this index has been removed and there is a new node at the same index. In addition to that, you also need to update the currentNode reference as it would still be pointing to the node you've just removed, but it should really be pointing to the new node that has moved to this index.

So in the if (item.equals(currentNode.getData())){ block you need to do the following:

Node nextNode = currentNode.getNextNode();
index++;
remove(i--);
currentNode = nextNode;

With this, your code should correctly remove all occurrences.

Answer 3

There is something special with your linked list, you can access next element with current.getNextNode but you delete using the element index. You should look in the rest of your implementation how this index is managed. Does the first element have index 0 or 1 (you start your loop with 1). What happens to the indexes of all elements when you remove one. Do the elements know their index ?

You could use something like

  int deletedNodes = 0;
  int currentIndex = 0; // check if 1 or 0
  currentNode = fist;
  while(currentNode != null){ // I guess lastNode.getNextNode() is null
    if(//should remove){
      remove(currentIndex);
      deletedNodes++
      // probably no need to change the index as all element should have been shifted back one index
    } else {
      currentIndex++; // index changes only if no node was deleted
    }
    currentNode = currentNode.getNextNode(); // will work even if it was deleted
  }
return deletedNodes;

Answer 4

I think the problem you have comes from remove(i).

When you remove the i-th element, the i+1-th element becomes the i-th and so on: every element is shifted. Therefore if you need to remove 2 elements in your list that are at index j and j+1, removing the j-th element calling remove(j) will shift the j+1-th element at the index j. Hence removing that second element requires calling remove(j) again, and not remove(j+1).

So you need to decrement i after removing.

Since your remove method actually decrements numberOfEntries, the condition on your while loop is properly updated. So all you need to do is replace

if (item.equals(currentNode.getData())) {
    index++;
    remove(i);
}
else {
    currentNode = currentNode.getNextNode();
} 

by

if (item.equals(currentNode.getData())) {
    index++;
    remove(i--);
}
// update the current node, whether removing it or not
currentNode = currentNode.getNextNode(); 

---

Iterator.remove()

This problem you are describing shows the usefulness of Iterator.remove() when using data structures from the JDK for going through an iterable collection and removing elements as you go through it.