patternjavaMinor
First Linked List in Java
Viewed 0 times
javalistfirstlinked
Problem
I just started learning Java, and wrote up a linked list. I'm coming from just having learned Haskell, so I've been out of the mindset of stateful computations for awhile.
I'd like to know how it could be improved in any way. Please be critical. Mainly though, I'd like suggestions on readability, and safe practices.
It's very simple right now, only allowing additions at the end and deletions; but I figured it would be sufficient for a review.
```
class Node {
T cargo;
Node tail;
public Node(T nCargo) {
cargo = nCargo;
tail = null;
}
public Node getNext() {
return tail;
}
public T getCargo() {
return cargo;
}
public boolean isLast() {
return tail == null;
}
public Node getLast() {
Node curNode = this;
while (!curNode.isLast()) {
curNode = curNode.getNext();
}
return curNode;
}
public void add(T nCargo) {
tail = new Node(nCargo);
}
public boolean canLink() {
return this.getNext().getNext() != null;
}
public void deleteNext() {
if (this.canLink()) { tail = tail.getNext();
} else { tail = null; }
}
}
public class Vect {
Node head;
int vSize;
public Vect() {
head = null;
vSize = 0;
}
public Vect(T cargo) {
head = new Node(cargo);
vSize = 1;
}
public void pushBack(T cargo) {
Node last = head.getLast();
vSize += 1;
last.add(cargo);
}
private Node getNodeAt(int i) {
Node curNode = head;
for (int cI = 0; i > cI; cI++) {
curNode = curNode.getNext();
}
return curNode;
}
public T getAt(int i) {
return getNodeAt(i).getCargo();
}
public int length() {
return vSize;
}
public void deleteHead() {
if (head.getNext() != null) { head = head.getNext();
} else { head = null; }
v
I'd like to know how it could be improved in any way. Please be critical. Mainly though, I'd like suggestions on readability, and safe practices.
It's very simple right now, only allowing additions at the end and deletions; but I figured it would be sufficient for a review.
```
class Node {
T cargo;
Node tail;
public Node(T nCargo) {
cargo = nCargo;
tail = null;
}
public Node getNext() {
return tail;
}
public T getCargo() {
return cargo;
}
public boolean isLast() {
return tail == null;
}
public Node getLast() {
Node curNode = this;
while (!curNode.isLast()) {
curNode = curNode.getNext();
}
return curNode;
}
public void add(T nCargo) {
tail = new Node(nCargo);
}
public boolean canLink() {
return this.getNext().getNext() != null;
}
public void deleteNext() {
if (this.canLink()) { tail = tail.getNext();
} else { tail = null; }
}
}
public class Vect {
Node head;
int vSize;
public Vect() {
head = null;
vSize = 0;
}
public Vect(T cargo) {
head = new Node(cargo);
vSize = 1;
}
public void pushBack(T cargo) {
Node last = head.getLast();
vSize += 1;
last.add(cargo);
}
private Node getNodeAt(int i) {
Node curNode = head;
for (int cI = 0; i > cI; cI++) {
curNode = curNode.getNext();
}
return curNode;
}
public T getAt(int i) {
return getNodeAt(i).getCargo();
}
public int length() {
return vSize;
}
public void deleteHead() {
if (head.getNext() != null) { head = head.getNext();
} else { head = null; }
v
Solution
Disclaimer: This review is reviewing this as work-in-process, and not as "finished".
On a quick glance this looks mostly correct. I still got a few points, that definitely need to be fixed before proceeding:
-
Index handling:
You don't do anything with negative indexes.
-
Empty list:
Your empty list throws null-pointer exceptions all around. Don't do that. You should check your head before doing stuff with it, and if it's null, throw
-
Optimizing
There is a relatively simple and quick-to-implement optimization for your
On a quick glance this looks mostly correct. I still got a few points, that definitely need to be fixed before proceeding:
-
Index handling:
You don't do anything with negative indexes.
vector.getAt(-1); results in an infinite loop, as well as vector.deleteAt(-1);. I suggest you just check and throw an InvalidArgumentException for any \$index -
Empty list:
Your empty list throws null-pointer exceptions all around. Don't do that. You should check your head before doing stuff with it, and if it's null, throw
IllegalStateException or similar. -
Optimizing
pushBack():There is a relatively simple and quick-to-implement optimization for your
pushBack. Your Vect should maintain a "pointer" to the last node. On deleting the last node you need to check for the special-case just like when deleting head, but after that your pushBack becomes dead-simple:public void pushBack(T cargo) {
tail.add(cargo);
tail = tail.getNext();
vSize++;
}Code Snippets
public void pushBack(T cargo) {
tail.add(cargo);
tail = tail.getNext();
vSize++;
}Context
StackExchange Code Review Q#70937, answer score: 3
Revisions (0)
No revisions yet.