Problem
I am solving some programming challenges for learning Javascript. The challenge I chose is:
Given the head of a singly linked list, reverse it in-place.
Here is my implementation in JS
// Given the head of a singly linked list, reverse it in-place.
const ReverseLinkedList = {
reverseLinkedList: (/** @type{ListNode} */ head) => {
if (head === undefined) {
return undefined;
}
if (head.next === undefined) {
return head;
}
let temp = head;
let tail = head.next.next;
head = temp.next;
head.next = temp;
head.next.next = undefined;
while (tail !== undefined) {
let tempNext = tail.next;
temp = head;
head = tail;
head.next = temp;
tail = tempNext;
}
return head;
},
ListNode: function ListNode(/** @type{ListNode} */ next, val) {
return {
next: next,
val: val
}
}
};
module.exports = ReverseLinkedList;
and my test class
const expect = require('chai').expect;
const reverseLinkedList = require('../../src/reverseLinkedList');
let head;
let node_01;
let node_02;
let reversed;
node_02 = reverseLinkedList.ListNode(undefined, 2);
node_01 = reverseLinkedList.ListNode(node_02, 1);
head = reverseLinkedList.ListNode(node_01, 0);
reversed = reverseLinkedList.reverseLinkedList(head);
expect(reversed.val).to.equal(2);
expect(reversed.next.val).to.equal(1);
expect(reversed.next.next.val).to.equal(0);
expect(reversed.next.next.next).to.equal(undefined);
node_01 = reverseLinkedList.ListNode(undefined, 1);
head = reverseLinkedList.ListNode(node_01, 0);
head.next = node_01;
reversed = reverseLinkedList.reverseLinkedList(head);
expect(reversed.val).to.equal(1);
expect(reversed.next.val).to.equal(0);
expect(reversed.next.next).to.equal(undefined);
head = reverseLinkedList.ListNode(undefined, 0);
reversed = reverseLinkedList.reverseLinkedList(head);
expect(reversed.val).to.equal(0);
expect(reversed.next).to.equal(undefined);
Being a Javascript newbie, any comments on the styling and best practices? Also any comments on the algorithm itself is highly appreciated.
Solution
I’m not a big fan of your type-hinting comments. Let JavaScript be JavaScript. If you want typing, practice TypeScript instead.
I find it counterintuitive that the ListNode
constructor takes its next
parameter before its val
parameter. Putting val
first is more natural (see my piDigits
example below), and also allows next
to be omitted for the last node in a list.
You can use the {val, next}
syntactic shorthand.
As for the reverseLinkedList
algorithm, there is too much code, with too many special cases. The fact that you mention .next.next
, reaching two elements ahead, is a sign that you aren’t thinking about the loop invariant the right way. You want the code to consider just the local situation at the head
.
It’s not really appropriate to explicitly set a variable to undefined
; undefined
is supposed to represent the fact that no assignment has taken place. In any case, you should be more lenient than checking for … === undefined
.
I find that it’s nearly always unhelpful to use temp
as a variable name or in a variable name.
const ListNode = (val, next) => { return {val, next}; };
const reverseLinkedList = (head) => {
let newTail = null;
while (head) {
let next = head.next;
head.next = newTail;
newTail = head;
head = next;
}
return newTail;
};
let piDigits = ListNode(3, ListNode(1, ListNode(4, ListNode(1, ListNode(5)))));
console.log(reverseLinkedList(piDigits));
let emptyList = null;
console.log(reverseLinkedList(emptyList));