views:

132

answers:

4

Hi. I need you to review my implementation of a Singly Linked List (SLL) please. The implementation should use generics and be able to use the enhanced for.

The problem is that, when I do for (Number n : list) being list a MyLinkedList<Integer> or MyLinkedList<Double>, I get the error: "Type mismatch: cannot convert from element type Object to Number".

This is what I have. The parts I am not very certain about are the generics and the iterators.

Thanks in advance.

import java.util.Iterator;

public class MyLinkedList<T> implements Iterable<Object>
{
    private Node head;

    public MyLinkedList ()
    {
        head = null;
    }

    public void add (Node n)
    {
        if (head == null)
        {
            head = n;
        }

        else
        {
            Node node = head;
            while (node.next != null) 
            {
                node = node.next;
            }
            node = n;
        }
    }

    public Iterator iterator() 
    {
        return new MyLinkedListIterator (head);
    }

    public int size () 
    {
        int ret = 0;
        MyLinkedListIterator it = new MyLinkedListIterator (head);
        while (it.hasNext ())
        {
            it.next();
            ret++;
        }

        return ret;
    }

    public Node getHead ()
    {
        return head;
    }
}

class MyLinkedListIterator<T> implements Iterator
{
    private Node node;

    public MyLinkedListIterator (Node h)
    {
        node = h;
    }

    public MyLinkedListIterator (MyLinkedList<T> l)
    {
        this(l.getHead ());
    }

    public boolean hasNext () 
    {
        if (node.next == null)
        {
            return false;
        }

        else
        {
            return true;
        }
    }

    public Object next () 
    {
        return node.next;
    }

    public void remove () 
    {

    }   
}
+2  A: 

You should return an Iterator<T> from the iterator method and you should also extend Iterable<T> instead of Iterable<Object>.

Besides, your MyLinkedListIterator<T> should implement Iterator<T>. Then it should work.

Ronald Wildenberg
+4  A: 
  • You should have Iterable<T> instead of Iterable<Object>.
  • add(Node) doesn't actually add an object to the list.
  • MyLinkedListIterator<T> should implement Iterator<T>.
  • MyLinkedListIterator.hasNext() will throw a NullPointerException if the list is empty.
  • MyLinkedListIterator.next() doesn't move to the next item in the list.
Steve Emmerson
A: 

Why dont you use <E>

public class Node<E>{
 E data;
 Node<E> next;
}

public class SinglyLinkedList<E> {

 Node<E> start;
 int size;
 .......
}

Look here for a comprehensive implementation

Bragboy
+1  A: 

On top of what the others have said, you probably shouldn't be exposing Node in your public methods - nodes should be a purely internal aspect of the implementation.

Andy