CODEWITHSUNDEEP
java
Fresh Java
Fresh Java

Reputation: 117

Singly linked list: wrong output after adding elements

I've implemented a singly linked list and I followed the Java documentation for the method. So I add 4 elements to the list(4 strings: A, B, C and D) and when I go through the list it only types out the last 3, starting from B that is. It skips the first element. Does it actually skip it or is the "head" just not refering to the next element?

@Override
public void add(E element) {
    Node<E> newNode = new Node<E>(element, null);
    if(head == null){
        head = newNode;
        tail = newNode;
    } else {
        tail.next = newNode;
        tail = newNode;
    }
}

EDIT: how I print out the elements: 

 MyList<String> list = new MyList<String>();

    list.add("A");
    list.add("B");
    list.add("C");
    list.add("D");

        for(String s : list){
            System.out.println(s);
        }

EDIT 2: 

public class MyList<E> implements SingleList<E> {

private Node<E> head;
private Node<E> tail;
private int size;

public MyList() {
    this.head = null;
    this.tail = null;
    this.size = 0;
}

private static class Node<E> {
    E data;
    Node<E> next;

    public Node(E data, Node<E> next) {
        this.data = data;
        this.next = next;
    }
}

private class iter implements Iterator<E> {
    Node<E> previous;
    Node<E> currentElement;

    public iter(){
        previous = null;
        currentElement = head;
    }

    @Override
    public boolean hasNext() {
        return currentElement.next != null;
    }

    @Override
    public E next() {
        if(! hasNext()){
            throw new NoSuchElementException("No more elements");
        }
        previous = currentElement;
        currentElement = currentElement.next;
        return currentElement.data;
    }

    @Override
    public void remove(){
        Node<E> remove = currentElement;
        currentElement = currentElement.next;
        previous.setNext(currentElement);
    }
}

@Override
public Iterator<E> iterator() {
    return new iter();
}

@Override
public void add(E element) {
    Node<E> newNode = new Node<E>(element, null);
    if(head == null){
        head = newNode;
        tail = newNode;
    } else {
        tail.next = newNode;
        tail = newNode;
    }
    size++;
}

Upvotes: 1

Views: 123

Answers (3)

Santiago Salem
Santiago Salem

Reputation: 577

Like puhlen told you, there was a problem with your next() method.

Documentation points that:

next() Returns the next element in the iteration.

So, if you are iterating through a list, the first node in the list should be return. Then second node and so on...

public E next() {
    if(! hasNext()){
        throw new NoSuchElementException("No more elements");
    }
    Node<E> actualNode = currentElement; //Save current node
    currentElement = currentElement.next; // Move to the next node
    return actualNode.data; //return the current node element
}

hasNext() Returns true if the iteration has more elements.

public boolean hasNext() {
    return currentElement.next != null;
}

Upvotes: 0

puhlen
puhlen

Reputation: 8519

Your iterator is wrong skips the first elements.

Notice that when you create the iterator, you set the currentValue to the head. But in the next(), you set current value to the next element, and then return that next element, instead of returning the current element. This causes the first value to be skipped, because the first call to next() will return head.next instead of the head.

Upvotes: 2

John Humphreys
John Humphreys

Reputation: 39224

This should work fine.

Are you sure you don't have an error in your printing function that is not using the first element properly?

Upvotes: 0

Related Questions