Return to Answer

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.

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.