Self-made Linked List

I was tasked by my teacher with making a linked list by myself, so I could understand the concept better. Here is the MyLinkedList class:

```
public class MyLinkedList{
private Node lastNode;
private int nodeCount;

public MyLinkedList(){
this.lastNode = null;
this.nodeCount = 0;
}

public int size() {
return nodeCount;
}

public boolean isEmpty() {
return this.nodeCount == 0;
}

public void add(T data) {
Node currentNode = new Node(data);

if (this.lastNode != null) {
currentNode.index = lastNode.index + 1;
currentNode.previousNode = lastNode;
lastNode.nextNode = currentNode;
}else {
currentNode.previousNode = null;
currentNode.index = 0;
}
this.lastNode = currentNode;
this.nodeCount++;
}

public T get(int index){
//error handling
if(this.isEmpty() || index this.nodeCount){
return null;
}
//
Node currentNode;
int count = lastNode.index - index;
currentNode = lastNode;

while (count > 0) {
currentNode = currentNode.previousNode;
count--;
}

return currentNode.data;
}

public Node getNode(int index){
//error handling
if(this.isEmpty() || index this.nodeCount){
return null;
}
//
int count = lastNode.index - index;
Node currentNode = lastNode;

while (count > 0){
currentNode = currentNode.previousNode;
count--;
}

return currentNode;
}

public boolean insert(T data, int index){
Node currentNode;

if (this.getNode(index) != null){
Node newNode = new Node(data);
currentNode = this.getNode(index);
newNode.index = index;

if (currentNode.previousNode != null) {
currentNode.

All things considered this looks like a really good beginner's attempt at the problem. The really good things I can see are:

• you have used generics

• it appears to all look functional

• it has advanced concepts (you have a double-linked list, not a single-linked list).

The generics look right, and the double-link is used well for locating data.

There are two major issues I see though that should be resolved though:

-
there is no need for the Node to be public. The Node should be a static inner class of the List, or a package-private (default) if the node is reused between different classes.

public class MyLinkedList {
    private static class Node {
        ....
    }
    ....

Note how the generic type of the node U is different to the list T, but that is because they are in different contexts, they are actually the same when you use the Node class like:

Node currentNode = new Node(data);

Because the Node is currently public, and especially because you 'leak' the node with the getNode(int) method, it is is possible of anyone to break your list, by getting a node, and then messing with the node internals.

-
The other concern is the index value in the node. It actually serves no purpose. You do not use it for anything, and you can do all the math you need based on the loop counters and list size. It is a lot of work to maintain, and it adds no value to the result.

You will find your code is much simpler if you resolve those two issues.

StackExchange Code Review Q#66685, answer score: 16