6
\$\begingroup\$

I'm new to JS and have I made this snippet for practicing. Because I have no idea for JS conventions, I want some advice about that.

function Queue() {
    this.list = new Array();

    this.pop = function() {
        return this.list.pop()
    };

    this.push =  function(el) {
        this.list.unshift(el);
    };

    this.size = function() {
        return this.list.length;
    };

    this.isEmpty = function() {
        return this.size() == 0;
    }

    this.print = function() {
        var res = '(';
        for (var i = 0; i < this.list.length-1; i++) {
            res += this.list[i] + ',';
        }
        res += this.list[this.list.length-1] + ')<br/>';
        document.write(res);
    }

    this.front = function() {
        return this.list[0];
    }

    this.back = function() {
        return this.list[this.list.length-1];
    }
}

/* test code */
var myQueue = new Queue();

myQueue.push(1);
myQueue.print();
myQueue.push(2);
myQueue.print();
myQueue.push(3);
myQueue.print();
myQueue.push(4);
myQueue.print();
myQueue.push(5);
myQueue.print();

myQueue.pop();
myQueue.print();
myQueue.pop();
myQueue.print();
myQueue.pop();
myQueue.print();
myQueue.pop();
myQueue.print();
myQueue.pop();
\$\endgroup\$
  • \$\begingroup\$ If your focus is on the JavaScript, I suggest you to remove the HTML around. \$\endgroup\$ – Zeta Feb 16 '17 at 7:59
  • 3
    \$\begingroup\$ It looks fine. I see no big improvementpotential. But besides that, you realized that you simply wrote a wrapper around a JS Array? To practice programming it is okay, but from a real-world point of view, I would rather use the array directly than an abstraction around it. It brings nothing new to the table (besides a "print" function). \$\endgroup\$ – Thomas Junk Feb 16 '17 at 8:44
7
\$\begingroup\$

Like Thomas Junk said, your wrapper pretty much maps 1:1 to JavaScript's existing, native Array. The biggest difference is that your push method maps to Array's unshift method, which, frankly, is very confusing.

push and pop - like shift and unshift - should be "symmetrical", so that the last thing that got pushed is what you get back when you call pop. But here (if the queue is longer than 1) you get something else back.

Typically, you'd make a queue with a plain [], and then push and shift elements on and off to get FIFO behavior.

You code does add a convenience methods for getting the first and last elements (the latter is indeed rather convenient), and a print. However, the print assumes you're running in a browser (not Node.js), and includes HTML in its output, which makes it less useful. And the logic differs little from the built-in string representation of an Array.

Besides, if you want to join all the elements in the array together into a string, there's join:

var res = "(" + this.list.join(",") + ")<br/>";

or the default coerced-to-string representation:

var res = "(" + this.list + ")<br/>";

Though again, neither one is terribly useful.

Here's how I'd make a queue:

var queue = [];

and here's how you'd use it:

queue.push(1);
queue.push(2);

// don't write to the document object for debugging purposes;
// just use the console
console.log(queue);

queue.shift(); // => 1
queue.shift(); // => 2
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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