HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Single Link List

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listsinglelink

Problem

So I have implemented Single link list using java.Please advice if this is OK or can be made better.

```
public class SingleLinkedList {

private Generic data;
private SingleLinkedList next;

public SingleLinkedList() {
super();
}

public SingleLinkedList(Generic data, SingleLinkedList next) {
super();
this.data = data;
this.next = next;
}

public Generic getData() {
return data;
}

public void setData(Generic data) {
this.data = data;
}

public SingleLinkedList getNext() {
return next;
}

public void setNext(SingleLinkedList next) {
this.next = next;
}

public SingleLinkedList add(Generic data, SingleLinkedList node) {
SingleLinkedList temp = new SingleLinkedList(data, null);
node.setNext(temp);
return temp;
}

public int size(SingleLinkedList node) {
int size = 0;
if (node == null || node.getNext() == null)
return 0;

// Get the first node
node = node.getNext();
do {
size++;
} while ((node = node.getNext()) != null);

return size;
}

public String toString(SingleLinkedList node) {

if (node == null || node.getNext() == null)
return "List Empty!";

// Get the first node
node = node.getNext();
StringBuilder data = new StringBuilder();
do {
data.append(node.getData() + ",");
} while ((node = node.getNext()) != null);

data.replace(data.length() - 1, data.length(), "");

return "SingleLinkedList [data=" + data.toString() + "]";
}

public boolean insertAtPosition(int position, Generic data, SingleLinkedList head) {

if (position temp;

// if position 1 then change and update head
if (position == 0) {
temp = new SingleLinkedList(data, null);

if (head.getNext() != null)
temp.setNext(head.getNext());

head.setNext(temp);

return true;
}

temp = head.getNext();
SingleLinkedList newNode = new SingleLinkedList<>(data, null);
while (temp != null && temp.getNext() != nu

Solution

It is customary to have a private Node class when implementing a list. This separates the holding of data from the managing of the list.

You on the other hand have both functionalities in one, which leads to all sorts of confusion.

For example, you expose the internal workings of the list by essentially returning nodes or working on nodes that are passed in from outside.

Your add method is also very confusing. It doesn't actually work on the list it is called on, but instead on a node passed to it. The same is true for size, but not for setNext.

I would completely rewrite your code. Start with a Node class that has nextNode and data as fields, and a SingleLinkedList class that has headNode and size as fields. Then add the methods you need, but keep in mind to not expose the Node class to the outside world.

Context

StackExchange Code Review Q#128174, answer score: 2

Revisions (0)

No revisions yet.