-2

I have a Bag class was given to me like looks like this:

import java.util.Iterator;
import java.util.NoSuchElementException;

public class Bag<Item> implements Iterable<Item> {

private int N;               // number of elements in bag
private Node<Item> first;    // beginning of bag

// helper linked list class
private class Node<Item> {
    private Item item;
    private Node<Item> next;
}

/**
 * Initializes an empty bag.
 */
public Bag() {
    first = null;
    N = 0;
}

/**
 * Is this bag empty?
 * @return true if this bag is empty; false otherwise
 */
public boolean isEmpty() {
    return first == null;
}

/**
 * Returns the number of items in this bag.
 * @return the number of items in this bag
 */
public int size() {
    return N;
}

/**
 * Adds the item to this bag.
 * @param item the item to add to this bag
 */
public void add(Item item) {
    Node<Item> oldfirst = first;
    first = new Node<Item>();
    first.item = item;
    first.next = oldfirst;
    N++;
}

public void remove(Item item){ 
    // currentNode is the reference to the first node in the list and to the Item
    Node<Item> currentNode = first; 
    // if items equals the first node in the list, then first = currentNode.next which will make the first item 
    Node<Item> temp = currentNode;
    while(temp.next != null){
        temp = currentNode;
        if(item.equals(currentNode.item)){
            currentNode = currentNode.next;
            temp.next = currentNode;
            break;
        }else{
            currentNode = currentNode.next;
        }
    }
    N--; 
}

/**
 * Returns an iterator that iterates over the items in the bag in arbitrary order.
 * @return an iterator that iterates over the items in the bag in arbitrary order
 */
public ListIterator<Item> iterator()  {
    return new ListIterator<Item>(first);  
}

// an iterator, doesn't implement remove() since it's optional
private class ListIterator<Item> implements Iterator<Item> {
    private Node<Item> current;

    public ListIterator(Node<Item> first) {
        current = first;
    }

    public boolean hasNext()  { return current != null;                     }
    public void remove()      { throw new UnsupportedOperationException();  }

    public Item next() {
        if (!hasNext()) throw new NoSuchElementException();
        Item item = current.item;
        current = current.next; 
        return item;
    }
}

I need to implement a remove method and the code written for it is mine. It does not work and I was wondering if someone could tell me why and how to correct it?

user3268401
  • 319
  • 1
  • 7
  • 21
  • The first thing to do is single-step through the code in an IDE debugger to see if you can spot where things start to go wrong. Do that before posting here. – Jim Garrison Feb 04 '14 at 23:18
  • I tried your code with changing Item class with String and it works. I think you should try again, it works. – Semih Eker Feb 04 '14 at 23:34

1 Answers1

2

A couple problems with your remove method.

  • You should only decrease N if you actually remove a node
  • If the bag contains duplicate items, should the remove() method remove all duplicates of that item? I would expect so.
  • You need to handle the case where you are removing the first node and update your first reference.
  • If you are not removing the first node, then you need to update the previous node to point to the next node after the node you are removing.

Bring it all together:

public void remove(Item item){
    Node<Item> currentNode = first;
    Node<Item> previousNode = null;
    while(currentNode != null){
        if(item.equals(currentNode.item)){
            if(previousNode  == null) {
              first = currentNode.next;
            }
            else {
              previousNode.next = currentNode.next;
            }
            N--;
        }
        else {
          previousNode = currentNode;
        }
        currentNode = currentNode.next;
    }
}
neildo
  • 2,206
  • 15
  • 12