1
\$\begingroup\$

Here's my Queue implementation in ES6,

class Queue{
    constructor(){
        this.head = null;
        this.tail = null;
    }
    offer(item){
        const p = new Node(item);
        if(!this.head){
            this.head = p;
            this.tail = p;
            return;
        }
        this.tail.next = p;
        this.tail = p;

    }
    poll(){
        const item = this.head;
        this.head = this.head.next;
        return item;

    }
    peek(){
        return this.head.val;
    }
    isEmpty(){
        return this.head === null;
    }
}

Please review on all fronts.

\$\endgroup\$

closed as off-topic by Heslacher, VisualMelon, IEatBagels, Graipher, Zeta Jan 24 at 5:46

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – IEatBagels, Graipher, Zeta
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Why is p assigned to both this.head and this.tail? What is the expected result? \$\endgroup\$ – guest271314 Jan 15 at 15:28
  • \$\begingroup\$ If that's the first node it's both the head and tail. The next node onwards offer moves the tail and poll moves the head. If I had one node and didn't assign both head and tail to it, the offer wouldn't work on the second node, since there's no tail defined yet. \$\endgroup\$ – Melissa Stewart Jan 15 at 17:05
  • 4
    \$\begingroup\$ Did you have a class definition for Node? If so, please edit your post to include it... otherwise this code appears broken, revealing an error after instantiating a Queue object and calling the offer method: Uncaught TypeError: Illegal constructor \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jan 22 at 23:07
3
\$\begingroup\$

Does not work, can be mutated

You did not even test this object. Please take time to make sure the code you put up for review is at least working next time. :)

Bugs.

You don't tests if head is defined for the functions peek, and poll.

const q = new Queue();
q.peek(); // throws TypeError cannot read property val of null
q.poll(); // throws TypeError cannot read property next of null

Can only Get one object from the Queue per group of offer calls

q.offer("A");
q.offer("B");
q.poll(); // return "A"
q.peek(); // throws TypeError cannot read property val of null
q.poll(); // throws TypeError cannot read property next of null


// OR
q.offer("A");
q.offer("B");
q.poll(); // return "A"
q.offer("C");
q.poll(); // return "C"  What happened to "B"

Mutation

You have not encapsulated the object state and thus can be mutated in many ways to produce unexpected behaviours

const q = new Queue()
q.offer("foo");
q.head.tail = q.head;  
while(!q.isEmpty()) { console.log(q.poll().val) } // never stops

// and
q.head = undefined;
q.isEmpty(); // incorrectly returns false

Bloated

The property Queue.tail does nothing and is just noise.

I am guessing you wanted to use it to make offer quicker without needing to step all the way down the links from head but that will require a doubly linked list.

You could just use an array.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.