patternjavaMinor
Swap Kth node from beginning with Kth node from end in a Linked List
Viewed 0 times
withnodeswapbeginninglistkthendfromlinked
Problem
Given a singly linked list, swap kth node from beginning with kth node from end. Swapping of data is not allowed, only pointers should be changed.
This code is attributed to geeksforgeeks. I'm looking for code-review, optimizations and best practices.
-
Why I don't extend or reuse: I am prepping for interviews, and interviewers explicitly want you to code, in my experience. I request the reviewer to not insist on reusing, as I am aware in real life reusability is the right approach. This does not work in interviews.
-
Why don't I use a
```
public class SwapKth {
private Node first;
private int size;
public SwapKth(List items) {
addAll(items);
}
private void addAll(List items) {
Node prev = null;
size = items.size();
for (T item : items) {
Node node = new Node(item);
if (prev == null) {
first = prev = node;
} else {
prev.next = node;
prev = node;
}
}
}
private static class Node {
private Node next;
private T item;
Node(T item) {
this.item = item;
}
}
public void swap (int n) {
if (n == 0) {
throw new IllegalArgumentException("The value of n should be greater than 0.");
}
if (n > size) {
throw new IllegalArgumentException("The value of n: " + n + " is greater than: " + size);
}
// code to reach the nth node from front.
Node x = first;
Node prevX = null;
for (int i = 0; x != null && i temp = x.next; // note: we have x.next in place.
Node y = first;
Node prevY = null;
for (; temp
This code is attributed to geeksforgeeks. I'm looking for code-review, optimizations and best practices.
-
Why I don't extend or reuse: I am prepping for interviews, and interviewers explicitly want you to code, in my experience. I request the reviewer to not insist on reusing, as I am aware in real life reusability is the right approach. This does not work in interviews.
-
Why don't I use a
Util class instead nesting method inside linked list? That is because I need the Node to be an internal data structure. Had I made a Util class, it would have no access to internal data structure and perform operations on the node's pointers.```
public class SwapKth {
private Node first;
private int size;
public SwapKth(List items) {
addAll(items);
}
private void addAll(List items) {
Node prev = null;
size = items.size();
for (T item : items) {
Node node = new Node(item);
if (prev == null) {
first = prev = node;
} else {
prev.next = node;
prev = node;
}
}
}
private static class Node {
private Node next;
private T item;
Node(T item) {
this.item = item;
}
}
public void swap (int n) {
if (n == 0) {
throw new IllegalArgumentException("The value of n should be greater than 0.");
}
if (n > size) {
throw new IllegalArgumentException("The value of n: " + n + " is greater than: " + size);
}
// code to reach the nth node from front.
Node x = first;
Node prevX = null;
for (int i = 0; x != null && i temp = x.next; // note: we have x.next in place.
Node y = first;
Node prevY = null;
for (; temp
Solution
Realize that when \$n > size / 2\$, it's equivalent to \$n = size - n + 1\$. That is, in a list of 6 elements, swapping with
To this:
This loop can be simplified a bit:
You don't need the null check on
In this code, you're effectively iterating from the beginning until the
But you don't have to: you have already iterated until the n-th, and to reach the \$(size - n + 1)\$-th node, you only need to iterate over \$size - 2 * n + 1\$ items, which can be far less than going from the beginning:
n=6 should be the same as n=1. As a result, this code can be simplified:Node prevFirst = null;
Node first = null;
Node prevSecond = null;
Node second = null;
if (n <= size/2) {
prevFirst = prevX;
first = x;
prevSecond = prevY;
second = y;
} else {
prevFirst = prevY;
first = y;
prevSecond = prevX;
second = x;
}
if (first.next == second) {
adjacentSwap(prevFirst, first, second);
} else {
distantSwap(prevFirst, first, prevSecond, second);
}To this:
if (n > size / 2) {
n = size - n + 1;
}
// ... the code finding `prevX`, `x`, `prevY`, `y`, `prevY`
if (x.next == y) {
adjacentSwap(prevX, x, y);
} else {
distantSwap(prevX, x, prevY, y);
}This loop can be simplified a bit:
for (int i = 0; x != null && i < (n - 1); x = x.next, i = i + 1) {
prevX = x;
}You don't need the null check on
x, thanks to the validation on n with respect to size at the beginning of the method. Also you should write ++i instead of i = i + 1, and no need for the parentheses in i < (n - 1):for (int i = 0; i < n - 1; x = x.next, ++i) {
prevX = x;
}In this code, you're effectively iterating from the beginning until the
size - n + 1-th node.Node temp = x.next; // note: we have x.next in place.
Node y = first;
Node prevY = null;
for (; temp != null; temp = temp.next, y = y.next) {
prevY = y;
}But you don't have to: you have already iterated until the n-th, and to reach the \$(size - n + 1)\$-th node, you only need to iterate over \$size - 2 * n + 1\$ items, which can be far less than going from the beginning:
Node y = x;
Node prevY = null;
for (int i = 0; i < size - 2 * n + 1; ++i, y = y.next) {
prevY = y;
}Code Snippets
Node<T> prevFirst = null;
Node<T> first = null;
Node<T> prevSecond = null;
Node<T> second = null;
if (n <= size/2) {
prevFirst = prevX;
first = x;
prevSecond = prevY;
second = y;
} else {
prevFirst = prevY;
first = y;
prevSecond = prevX;
second = x;
}
if (first.next == second) {
adjacentSwap(prevFirst, first, second);
} else {
distantSwap(prevFirst, first, prevSecond, second);
}if (n > size / 2) {
n = size - n + 1;
}
// ... the code finding `prevX`, `x`, `prevY`, `y`, `prevY`
if (x.next == y) {
adjacentSwap(prevX, x, y);
} else {
distantSwap(prevX, x, prevY, y);
}for (int i = 0; x != null && i < (n - 1); x = x.next, i = i + 1) {
prevX = x;
}for (int i = 0; i < n - 1; x = x.next, ++i) {
prevX = x;
}Node<T> temp = x.next; // note: we have x.next in place.
Node<T> y = first;
Node<T> prevY = null;
for (; temp != null; temp = temp.next, y = y.next) {
prevY = y;
}Context
StackExchange Code Review Q#60202, answer score: 5
Revisions (0)
No revisions yet.