My LinkedList Iterator - Can't print first element - No use of Java API

[Dante Hawke]

Hello all,

I am currently trying to create my own Iterator for my own LinkedList class as part of a task, it's implementation must mirror that of the Java API LinkedList.

My BasicLinkedList works fine, however I have become a bit stuck working on its anonymous Iterator class, that should have a hasNext(), next() and remove() method.
At the moment it can hasNext() and print out the next() of all items in the list except for the first.

Here is my code.

BasicLinkedList:

import java.lang.Iterable; // Import the Iterable package.
import java.util.Iterator; // Import the Iterator package.

public class BasicLinkedList<T> implements, Iterable<T> {	
	
	private Node header = null; // Reference variable for the header node.
	private Node curNode = null; // Reference variable for the current node.
	private int size = 0; // Set the size of the BasicLinkedList to 0.
			
	public boolean add(T item) {
		curNode = header; // Set reference variable to point to header.
		Node temp;
		if(item != null) { // If item is not null.
			if(header != null) { // And If header is not equal to null.
				while(curNode.getNext() != null) { // While this curNode.getNext is not null.
					curNode = curNode.getNext(); // Get me to the end.
				}
				curNode.setNext(new Node (item)); // Set the last Node to point to the new Node, 
												      // which contains the item I want to add.												
			} else if (header == null) {
				temp = new Node(item);
				header = temp;
			}
			size++; // Increment size of BasicLinkedList by 1.
		}
		return true;
	}
	
	public boolean remove(T item) {
		Node tempNode;
		curNode = header;
		tempNode = curNode;
		
		if(curNode.getItem().equals(item)) {
			header = curNode.getNext();
			curNode = null;
			size--;
			return true;
		}
		
		while(curNode != null) {
			curNode = curNode.getNext();
			if(curNode.getItem().equals(item)) {
				tempNode.setNext(curNode.getNext());
				curNode = null;
				size--; // Decrease size of SimpleLinedList by 1.
				return true;
			}
			tempNode = tempNode.getNext();
		}
		return false;		
	}	
}

Anonymous Iterator Class:

	/**
	 * An anonymous class of Iterator that provides an iterator to go through the whole asicLinkedList.
	 * 
	 * @return Iterator
	 */
	@Override
	public Iterator<T> iterator() {
		// Anonymous class defined as part of the return statement
		Iterator<T> iterator = new Iterator<T>() {
		private Node current = header;		
			
			public boolean hasNext() { // True if there is a next Object in the SimpleLinkedList.
				return current != null && current.next != null;
			}
			
			public T next() { // Return the object.
				if (!hasNext()) {
					throw new java.util.NoSuchElementException();
				}				
				current = current.getNext();				
				return current.getItem();
			}
			
			public void remove() { // Removes the most recent element that was returned by the next() method.
				throw new UnsupportedOperationException();
			}
		};
		return iterator; // Return an iterator that has just been called.
	}	

Inner Node Class:

private class Node {

		private T item; // Holds the generic Object in the Node.
		private Node next; // A reference to the Node's next Node in the BasicLinkedList.
		
		/**
		 * Default Constructor for a Node.
		 */
		public Node() {
			this.item = null;
			this.next = null;
		}
		
		/**
		 * Constructor for a new Node.
		 * 
		 * @param item - The Object to be held in this Node.
		 */
		public Node(T item) {
			this.item = item;
			this.next = null;
		}		

		/**
		 * getItem - Return the item held in this Node.
		 * 
		 * @return item - T in this Node.
		 */
		public T getItem() {
			return item;
		}

		/**
		 * getNext - Return the next node in the list.
		 * 
		 * @return next - Reference to the next Node.
		 */
		public Node getNext() {
			return next;
		}
		
		/**
		 * setNext - Set the next Node in the list.
		 * 
		 * @param next - Reference to the next Node.
		 */
		public void setNext(Node next) {
			this.next = next;
		}
		
	}

Any help is appreciated, many thanks!

Cheers.

[Lorand Komaromi]

Dante Hawke wrote:private Node current = header;

You have to make sure that the first call to next() will return the first Node in the List. You could achieve this by setting current to a new Node instance whose next you would set to head...

[Dante Hawke]

Many thanks, that solved that problem, but now it doesn't print the last one in the LinkedList, I'm completely stumped here!

/**
	 * An anonymous class of Iterator that provides an iterator to go through the whole SimpleLinkedList.
	 * 
	 * @return Iterator
	 */
	@Override
	public Iterator<T> iterator() {
	// Anonymous class defined as part of the return statement
		Iterator<T> iterator = new Iterator<T>() {
			private Node temp1 = null; // Reference variable to a temporary node.
			private Node current = header;				
			
			public boolean hasNext() { // True if there is a next Object in the SimpleLinkedList.
				return current != null && current.next != null;
			}
			
			public T next() { // Return the object.
				if (!hasNext()) {
					throw new java.util.NoSuchElementException();
				}	
				temp1 = current;
				current = current.getNext();				
				return temp1.getItem();
			}
			
			public void remove() { // Removes the most recent element that was returned by the next() method.
				current.setNext(current.getNext());
			}
		};
		return iterator; // Return an iterator that has just been called.

[Campbell Ritchie]

Are you sure about current.setNext(current.getNext())? Does that really remove the item?

[Dante Hawke]

My hasNext() and next() method appear to work perfectly now under a few simple tests, my refined Iterator is as follows:

My current remove method is:

public Iterator<T> iterator() {
	// Anonymous class defined as part of the return statement
		Iterator<T> iterator = new Iterator<T>() {
			private Node temp2 = null; // Reference variable to a temporary node.
			private Node current = header;				
			
			public boolean hasNext() { // True if there is a next Object in the SimpleLinkedList.
				//return current != null && current.next != null;
				if(current == null) {
					return false;
				}
				else {
					return true;
			    }			
			}
			
			public T next() { // Return the object.
				if (!hasNext()) {
					throw new java.util.NoSuchElementException();
				}	
				temp2 = current;
				current = current.getNext();				
				return temp2.getItem();
			}
			
			public void remove() { // Removes the most recent element that was returned by the next() method.
				if(current == header){
					current.setNext(temp2.getNext());
				}
				else {
					temp2.setNext(current.getNext());
				}
								
			}
		};
		return iterator; // Return an iterator that has just been called.
	}

However, with some testing I have found that the remove method will always fail to remove the first Node.

[Campbell Ritchie]

That looks better, though I haven't gone through it carefully