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

Delete alternate nodes of a linked list

Submitted by: @import:stackexchange-codereview··
0
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;

Solution

Unused Variables / wrong variable values

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

  • create should probably be called add



  • n would be better as node or newNode (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.