Implementing a Singly Linked List

I am implementing, in my SinglyLinkedList class, an interface where I redefined linked list. I am trying to learn data structures. Please, let me know what I can approve upon in my code.

```
public class SinglyLinkedList implements LinkedList {

private class Node {

public Node next;
public E element;

public Node(E element) {

this.element = element;
}

public Node (E element, Node next) {

this.element = element;
this.next = next;
}

public E getElement() {

return element;
}

public Node getNext() {

return next;
}

public void setElement(E element) {

this.element = element;
}

public void setNext(Node next) {

this.next = next;
}

public String toString() {

return ("[" + element + "] ");
}
}

public Node head;
public Node tail;
public int total;

public SinglyLinkedList() {

this.head = null;
this.tail = null;
this.total = 0;
}

public E get(int index) {

if (index size()) {
return null;
}

if (index == 0) {
return head.getElement();
}

Node singly = head.getNext();

for (int i = 1; i singlyAdd = new Node(element);

if (tail == null) {
head = singlyAdd;
tail = singlyAdd;
} else {
tail.setNext(singlyAdd);
tail = singlyAdd;
}

total++;
}

public boolean add(int index, E data) {

if (index size()) {
return false;
} else {
Node singlyAdd = new Node(data);
Node singly = head;

for (int i = 1; i current = head;
while (current != null) {
System.out.print(current.toString());
current = current.get

Interface

Your interface differs from the standard LinkedList interface in a couple of points:

• add(E) returns nothing: The standard function returns true if the list changed, but I think your way is fine as well.

• add(int, E): Your function returns false, while the standard throws an exception. This is an exceptional case, so the standard interface handles this better than your solution.

• you never throw any exceptions when things are out of bounds. Eg, I can remove a node at an index that doesn't exist, and your code does this without notifying me that it doesn't do anything. I would have to check what your method returned to find out. Exceptions are the better solution here.

So I would replace all your

if (index  size()) {
        return ;
    }

with

if (index  size()) {
        throw new IndexOutOfBoundsException(index + " is not a valid index. List length is only " + size());
    }

Misc

• fields should be private, and the access methods should be used to access them.

• do not print in a class. If you were to actually use this list in projects, you don't want it to print anything.

• you don't need the null checks (singly.getNext() != null, if (singly.getNext() == null)) in your loops, as you already checked the bounds at the beginning.

• use @Override to show which methods implement the interface, and which don't.

StackExchange Code Review Q#87274, answer score: 3