Is using closures to emulate encapsulation a bad idea?

closures, javascript

Solution

Is using closures to emulate encapsulation a bad idea?

No. Although I wouldn't consider this to be "emulation", closures are implementing encapsulation.

I did try to optimize it by making use of prototypes instead of creating functions every time a new object is created. In other words, I didn't include the functions as part of the constructor of MyStack.

My question is, is this poor design? Are there are any major downfalls to this methodology?

Yes, that's actually wrong. Your `min` and `head` (and `MyStack` and `Node`) variables are essentially static. They are defined only once, and will be shared by all instances. You cannot create two distinct stacks, they both will have the same `head` reference.

To encapulate per-instance state, you will need to declare the variables in the constructor so that they will be created with every new object. For that, you'll have to also declare all methods that need to access them ("privileged") in the constructor scope.

var MyStack = (function() {
    function MyStack() {
        var size = 0;
        var head = undefined;

        function checkNonEmpty() {
            if (typeof head === 'undefined') {
                throw new Error('Empty stack');
            }
        }
        this.push = function(val) {
            size++;
            head = new Node(val, head);
        };
        this.pop = function() {
            checkNonEmpty();
            this.size--;
            var data = head.data;
            head = head.next;
            return data;
        };
        this.peek = function() {
            checkNonEmpty();
            return head.data;
        };
        this.getSize = function() {
            return size;
        };
    }

    function Node(data, next) {
        this.data = data;
        this.next = next;
    }

    return MyStack;
})();

If you want to put these methods on the prototype, you will need to make the `head` and `size` values available as properties of the instance.

Problem

For example, take a look at my simple implementation of a stack: ``` var MyStack = (function() { var min; var head; // Constructor function MyStack() { this.size = 0; } MyStack.prototype.push = function(val) { var node = new Node(val); if (typeof min === 'undefined' || val < min) { min = val; } ++this.size; if (typeof head === 'undefined') { head = node; } else { node.next = head; head = node; } }; MyStack.prototype.pop = function() { if (typeof head === 'undefined') { throw new Error('Empty stack'); } --this.size; var data = head.data; head = head.next; return data; }; MyStack.prototype.min = function() { if (typeof min === 'undefined') { throw new Error('Min not defined'); } return min; }; MyStack.prototype.peek = function() { if (typeof head === 'undefined') { throw new Error('Empty stack'); } return head.data; }; function Node(data) { this.data = data; this.next; } return MyStack; })(); ``` By using this method, I can make sure that no one is able to (accidentally or intentionally) manipulate "private" fields such as min and head. I can also make use of private functions such as Node() which doesn't need to be exposed. I have read that this will use more memory just because of the fact that it has to maintain an additional scope for each new object that is created for MyStack. Does it need so much extra memory that this way is a bad idea? I did try to optimize it by making use of prototypes instead of creating functions every time a new object is created. In other words, I didn't include the functions as part of the constructor of MyStack. My question is, is this poor design? Are there are any major downfalls to this methodology?

Original source