Stack Implementation in JavaScript

Question

I am learning data structures and below is my implementation of a stack in JavaScript. I didn't want to use built in functions for an array because in JavaScript that would be too easy, so I made my own.

class Stack {

  constructor(data=null) {
    this.items = [data]
    this.length = 0
  }

  push(data) {
    this.items[this.length] = data
    this.length++
  }

  pop() {
    delete this.items[this.length-1]
    this.length--
  }

  printStack() {
    for(let i = 0; i<this.length; i++)
      console.log(this.items[i])
  }

  isEmpty() {
    return (this.length) > 0 ? false : true
  }

}

let myStack = new Stack();

myStack.push(123)
myStack.push(456)
myStack.pop()

myStack.printStack()

console.log(myStack.isEmpty())

Answer 1

You have a bug in your constructor:

constructor(data=null) {
    this.items = [data]
    this.length = 0
  }

If you pass an item and then push to the stack, you lose the first item:

let myStack = new Stack("123");
myStack.push("456")
// myStack.items = ["456"]

You need to make sure that you initialize length to 1 if an item is passed in. The simplest way to do that is something like this:

constructor(datum = null) {
    if (datum !== null) {
        this.items = [datum];
        this.length = 1;
    } else {
        this.items = [];
        this.length = 0;
    }
}

Some people like to prefix private fields with _ to show that they aren't supposed to be used externally. You could consider using private names but they aren't supported everywhere.

Your pop function should return the element that was popped from the stack to be useful.

You can also simplify isEmpty:

isEmpty() {
    return this.length === 0;
}

Answer 2

Encapsulate

The object Stack should protect its state as it is easy to add and remove items using via the exposed referenced Stack.items, or when setting Stack.length

You can never guarantee that it will act like a stack if you can not trust its state.

Review

Most points are already covered by accepted answer, however there are some additional points.

• Avoid using null

• Stack.pop does not return anything? What is the point of a stack that does not provide access to the top item.

• Use getters and setters rather than give direct access to state critical properties.

• Rather than Stack.printStack, use a getter to get a copy of the stack that you can then use to log to the console or anything else.

• Spaces between operators. eg [this.length - 1] not [this.length-1]

• Always delimit code blocks. for( ; ; ) /* code */ should be for( ; ; ) { /* code */ }

• Avoid redundant or long form code.

  You had delete this.items[this.length-1]; this.length-- could have been a single line delete this.items[--this.length]

Rewrite

The rewrite creates a stack using a factory function. The Stack is frozen to ensure it's state can be trusted.

function Stack(...items) {
    const stack = items;
    return Object.freeze({
        push(...items) { stack.push(...items) },
        pop() { return stack.pop() },
        clear() { stack.length = 0 },
        get size() { return stack.length },
        get isEmpty() { return !stack.length },
        get items() { return [...stack] },
    });         
}


// usage
const myStack = Stack(1, 2, 3);
myStack.push(123, 436, 512);
console.log(myStack.items.join(","));
while (!myStack.isEmpty) { 
    console.log("Pop item[" + (myStack.size - 1) + "]: "  + myStack.pop());
}