skip to Main Content

I have this singly linked list:

class Node{
    constructor(val){
        this.val = val;
        this.next = null;
    }
}

class SinglyLinkedList{
    constructor(){
        this.head = null;
        this.tail = null;
        this.length = 0;
    }
}

and this shift method:

    shift(){
        if(!this.head) return undefined;
        var currentHead = this.head;
        this.head = currentHead.next;
        this.length--;
        if(this.length === 0){
            this.tail = null;
        }
        return currentHead;
    }

The question is whether currentHead.next should be equal to null or not. I found several sources on the internet proposing the shift method from above, but I feel like the correct version is with currentHead.next = null.

2

Answers


  1. The reference to currentHead will be returned, and since it is no longer referenced by the list itself, there is no need to set currentHead.next to null, because JavaScript’s garbage collector will clean up when it isn’t unused anymore.
    But, if you want to ensure the detached node is completely isolated (i.e., its next property points to null), you could add currentHead.next = null;. This will prevent memory leaks or unintended side effects if the returned node is used once returned.

    I guess you could also return the value from the currendHead instead of the Node.

    Login or Signup to reply.
  2. Setting the next property to null will avoid that the caller would follow that link and traverse through the list, which really is the task of the SinglyLinkedList class and not of the caller.

    But to bring that point home, you should really not return a node, but just the value. The caller should have no business with the Node class. It should only be the SinglyLinkedList class that uses it for its logic, but besides that it should better be completely hidden from the caller. To the caller the linked list should be nothing more than a collection of values and methods to delete or insert values.

    So with that in mind, you would design your classes as follows:

    class SinglyLinkedList {
        // Make properties private: caller has no business with those
        #head = null;
        #tail = null;
        #length = 0;
    
        static Node = class {
            constructor(val) {
                this.val = val;
                this.next = null;
            }
        }
        
        constructor(...values) { // allow to be initialised with values
            this.push(...values);
        }
        
        push(...values) {
            for (const val of values) {
                if (!this.#length++) {
                    this.#tail = this.#head = new SinglyLinkedList.Node(val);
                } else {
                    this.#tail = this.#tail.next = new SinglyLinkedList.Node(val);
                }
            }
        }
        
        shift() {
            if (!this.#head) return;
            const val = this.#head.val; // Get value only
            this.#head = this.#head.next;
            if (!--this.#length) this.#tail = null;
            return val;
        }
        
        *values() {
            for (let node = this.#head; node; node = node.next) {
                yield node.val; // only yield value, not node
            }
        }
        
        *[Symbol.iterator]() {
            yield* this.values();
        }
        
        toString() {
            return [...this.values()].join(" -> ");
        }
    }
    
    const list = new SinglyLinkedList(10, 20, 30, 40);
    console.log(...list);
    const first = list.shift();
    console.log("shifted out:", first);
    console.log("remainder: " + list);
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search