patternjavaMinor
Singly linked list with generics and comparable data - corrected code
Viewed 0 times
withcomparabledatasinglycorrectedcodegenericsandlistlinked
Problem
I am currently reading about the thread-safe implementation of a linked list. I would like someone to point out some mistakes that will create issues if I try to make it thread-safe in the future.
I've corrected the buggy code from my original post.
Information:
```
package com.atleyvirdee.myDataStructures.linkedlist.implemtations;
import com.atleyvirdee.myDataStructures.linkedlist.ILinkedList;
import com.atleyvirdee.myDataStructures.linkedlist.ILinkedListNode;
import com.atleyvirdee.myDataStructures.linkedlist.LinkedListerTraverser;
public class SinglyLinkedList> implements ILinkedList {
ILinkedListNode root = null;
ILinkedListNode last = null;
int size = 0;
LinkedListerTraverser traverser = new LinkedListerTraverser(root, size);
@Override
public ILinkedListNode getRoot() {
return root;
}
@Override
public LinkedListerTraverser getTraverser() {
traverser.setRoot(root, size);
return traverser;
}
@Override
public void prepend( T data ) {
if ( this.root == null ) {
append(data);
return;
}
size++;
ILinkedListNode node = new NodeImpl(data);
node.setNext(this.root);
this.root = node;
}
@Override
public void append( T data ) {
size++;
ILinkedListNode node = new NodeImpl(data);
if ( this.root == null ) {
this.root = node;
this.last = node;
return;
}
this.last.setNext(node);
this.last = node;
}
@Override
public void remove( T data ) {
ILinkedListNode pare
I've corrected the buggy code from my original post.
Information:
- Please review the
SinglyLinkedListmainly.
- You may point out any design issue in classes or suggest another function that should be implemented.
- I have written the code for practice purpose only. I know its not Java-linked-list compatible.
- I tried to apply all the test cases. Please don't answer if you found another bug; just mention it in a comment. I will introduced it in test cases then get reviewed.
SinglyLinkedList:```
package com.atleyvirdee.myDataStructures.linkedlist.implemtations;
import com.atleyvirdee.myDataStructures.linkedlist.ILinkedList;
import com.atleyvirdee.myDataStructures.linkedlist.ILinkedListNode;
import com.atleyvirdee.myDataStructures.linkedlist.LinkedListerTraverser;
public class SinglyLinkedList> implements ILinkedList {
ILinkedListNode root = null;
ILinkedListNode last = null;
int size = 0;
LinkedListerTraverser traverser = new LinkedListerTraverser(root, size);
@Override
public ILinkedListNode getRoot() {
return root;
}
@Override
public LinkedListerTraverser getTraverser() {
traverser.setRoot(root, size);
return traverser;
}
@Override
public void prepend( T data ) {
if ( this.root == null ) {
append(data);
return;
}
size++;
ILinkedListNode node = new NodeImpl(data);
node.setNext(this.root);
this.root = node;
}
@Override
public void append( T data ) {
size++;
ILinkedListNode node = new NodeImpl(data);
if ( this.root == null ) {
this.root = node;
this.last = node;
return;
}
this.last.setNext(node);
this.last = node;
}
@Override
public void remove( T data ) {
ILinkedListNode pare
Solution
I'll just focus on one method:
As pointed out in my answer to another of your questions, requiring the nodes to be
Naming interfaces with the
For a linked list, the conventional terminology is "head" instead of "root".
The only way that the last line is reachable is if
@Override
public T find( T data ) {
ILinkedListNode node = root;
while (node != null) {
if ( node.getData().compareTo(data) == 0 ) {
return node.getData();
}
node = node.getNext();
}
return (node == null) ? null : node.getData();
}As pointed out in my answer to another of your questions, requiring the nodes to be
Comparable is an unreasonable burden. If all you want to check is equality, then call node.getData().equals(data) — every Java object has a .equals() method.Naming interfaces with the
I… Hungarian prefix is unconventional for Java; it's more appropriate in C#.For a linked list, the conventional terminology is "head" instead of "root".
The only way that the last line is reachable is if
node is null. You don't need to test the while-loop condition again. I suggest writing it as a for loop instead, to make the pattern more easily recognizable:@Override
public T find(T data) {
for (LinkedListNode node = head; node != null; node = node.getNext()) {
if (node.getData().equals(data)) {
return node.getData();
}
}
return null;
}Code Snippets
@Override
public T find( T data ) {
ILinkedListNode<T> node = root;
while (node != null) {
if ( node.getData().compareTo(data) == 0 ) {
return node.getData();
}
node = node.getNext();
}
return (node == null) ? null : node.getData();
}@Override
public T find(T data) {
for (LinkedListNode<T> node = head; node != null; node = node.getNext()) {
if (node.getData().equals(data)) {
return node.getData();
}
}
return null;
}Context
StackExchange Code Review Q#96697, answer score: 3
Revisions (0)
No revisions yet.