Recursive methods of singly linked list

Can anyone review the methods? If there are better ways to write a certain method I would appreciate it if anyone could tell me.

```
public class Node {
E data;
Node next;

public Node(E item) {
setData(item);
setNext(null);
}

public Node(E item, Node Ref) {
setData(item);
setNext(Ref);
}

public E getData() {
return data;
}

public void setData(E item) {
data = item;
}

public Node getNext() {
return next;
}

public void setNext(Node Ref) {
next = Ref;
}
}

class MySinglyLinkedList {
// attributes
private int size;
private Node head;

// methods
public MySinglyLinkedList() {
size = 0;
head = null;
}

// Method size
private int sizeHelper(Node head) { // Helper Method
if (head == null)
return 0;
else
return 1 + sizeHelper(head.next);
}

public int size() { // Wrapper Method
return sizeHelper(head);
}

// Method add
private boolean addHelper(Node head, E data) { // Helper Method
if (head.next == null) {
head.next = new Node(data);
return true;
} else
return addHelper(head.next, data);
}

public boolean add(E data) { // Wrapper Method
if (head == null) {
head = new Node(data);
return true;
} else
return addHelper(head, data);
}

// Method toString
private String toStringHelper(Node head) { // Helper Method
if (head == null)
return "";
else
return head.data + "\n" + toStringHelper(head.next);
}

public String toString() { // Wrapper Method
return toStringHelper(head);
}

// Method get
private Node getNodeHelper(int i, Node head) { // Helper Method
if (i == 0)
return head;
else
return getNodeHelper(i - 1, he

The code that you have is clear, properly indented and easily readable. Internal methods are private while the public API is public, which is a very good thing.

I have a handful of comments:

• You have a member private int size; but it is unused at the moment. You might want to consider removing it - unused code should be deleted. However, a better alternative would be to use it as a cache for the size() method. In the add() method, you can increment it by one when an object is added, similarly, in the remove() method, you can decrement it by one when an object is removed.

-
You should try to always write the curly braces surrounding your if / else statements, even if it might be unnecessary. It makes the code less error-prone for the future. For example, in the following code

if (head == null)
    return 0;
else
    return 1 + sizeHelper(head.next);

you should have

if (head == null) {
    return 0;
} else {
    return 1 + sizeHelper(head.next);
}

or even

if (head == null) {
    return 0;
}
return 1 + sizeHelper(head.next);

which makes the code slightly shorter (although this might be a matter of opinion).

-
Your indexOf method returns a boolean.

public boolean indexOf(E data)

this might be confusing for the callers, which would be less surprised to have that method return an int, like the standard List.indexOf(...). Consider updating that method to make it return the int representing the index of the given element, or -1 if the element isn't found.

• Your insert(element, i) element is also surprising: one would expect this method to insert the given element at the index given, like the standard List.add(index, element), but your method appends a node having the value i instead. Note that you're using a raw type in this method with new Node(i, null);, which you should never do.

StackExchange Code Review Q#125961, answer score: 2