patternjavaMinor
Delete alternate nodes of a linked list
Viewed 0 times
nodesdeletelistlinkedalternate
Problem
If the linked list is 1->2->3->4 then the output should be 1->3. If the linked list is 1->2->3->4->5, then the output should be 1->3->5.
The question is attributed to GeeksForGeeks. I'm looking for code-review, best practices and optimizations.
```
public class DeleteAlternate {
private Node first;
private Node last;
private int size;
public DeleteAlternate(List items) {
for (T item : items) {
create(item);
}
}
private void create (T item) {
Node n = new Node<>(item);
if (first == null) {
first = last = n;
} else {
last.next = n;
last = n;
}
size++;
}
private final class Node {
private Node next;
private T item;
Node (T item) {
this.item = item;
}
}
public void deleteAlternate ( ) {
if (first == null) {
throw new IllegalStateException("The first node is null.");
}
Node node = first;
// node == null, if even nodes are present in LL
// node.next == null, if odd nodes are present in LL
while (node != null && node.next != null) {
node.next = node.next.next;
node = node.next;
}
}
// size of new linkedlist is unknown to us, in such a case simply return the list rather than an array.
public List toList() {
final List list = new ArrayList<>();
if (first == null) return list;
for (Node x = first; x != null; x = x.next) {
list.add(x.item);
}
return list;
}
@Override
public int hashCode() {
int hashCode = 1;
for (Node x = first; x != null; x = x.next)
hashCode = 31*hashCode + x.hashCode();
return hashCode;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
The question is attributed to GeeksForGeeks. I'm looking for code-review, best practices and optimizations.
```
public class DeleteAlternate {
private Node first;
private Node last;
private int size;
public DeleteAlternate(List items) {
for (T item : items) {
create(item);
}
}
private void create (T item) {
Node n = new Node<>(item);
if (first == null) {
first = last = n;
} else {
last.next = n;
last = n;
}
size++;
}
private final class Node {
private Node next;
private T item;
Node (T item) {
this.item = item;
}
}
public void deleteAlternate ( ) {
if (first == null) {
throw new IllegalStateException("The first node is null.");
}
Node node = first;
// node == null, if even nodes are present in LL
// node.next == null, if odd nodes are present in LL
while (node != null && node.next != null) {
node.next = node.next.next;
node = node.next;
}
}
// size of new linkedlist is unknown to us, in such a case simply return the list rather than an array.
public List toList() {
final List list = new ArrayList<>();
if (first == null) return list;
for (Node x = first; x != null; x = x.next) {
list.add(x.item);
}
return list;
}
@Override
public int hashCode() {
int hashCode = 1;
for (Node x = first; x != null; x = x.next)
hashCode = 31*hashCode + x.hashCode();
return hashCode;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
Solution
Unused Variables / wrong variable values
You are not actually using
You should do this because the assumption will always be that these values are correct, and as they are not, this could easily lead to bugs in the future.
Naming
hashCode and equals
I said this already, but the list implements
In
Tests
As this is a generic list, I would not only test primitive types, but also custom classes.
You are not actually using
last for the algorithm, and you are also not keeping it up-to-date, so I would just get rid of it. Alternatively, keep it, but then it should always have a correct value. The same goes for size. You should do this because the assumption will always be that these values are correct, and as they are not, this could easily lead to bugs in the future.
Naming
createshould probably be calledadd
nwould be better asnodeornewNode(it is a small scope, so it's not that bad)
- your test cases should have proper names, so it is obvious what went wrong from looking at the name.
hashCode and equals
I said this already, but the list implements
hashCode and uses the hashCode method of your Node class, but that class does not implement hashCode.In
equals: Do not just use !=, but use the equals method of your node, which in turn should use the equals method of the item. If you use non-primitive types, your current approach will lead to bugs.Tests
As this is a generic list, I would not only test primitive types, but also custom classes.
Context
StackExchange Code Review Q#59594, answer score: 7
Revisions (0)
No revisions yet.