1

How do I grant access to inner properties of objects in the right way? This is what does break my application:

I have an object that handles an array (simplified here):

function ListManager() {
    var list = [],
        add = function (element) {
            list.push(element);
        },
        clear = function () {
            list = [];
        };

    return {
        add: add,
        clear: clear,
        list : list
    };
};

But I get this when using it:

var manager = new ListManager();
manager.add("something");
manager.clear();
console.log(manager.list.length); // <= outputs "1"!

Stepping through the code shows, that within the clear method, list becomes a new array. But from outside the ListManager the list ist not cleared.

What am I doing wrong?

Michael Hilus
  • 1,647
  • 2
  • 21
  • 42
  • You need to use `this.list` in clear method. – Rahil Wazir May 29 '14 at 23:00
  • 1
    On returning the list on the list property, I think its passing the value instead of reference, that's you are getting that value!! – prabeen giri May 29 '14 at 23:06
  • It is indeed a by-reference / by-value problem. – Michael Hilus May 29 '14 at 23:26
  • You guys who marked this a dup have an odd sense of what a duplicate is. Just because a concept involved in this solution is discussed in another answer is NOT sufficient to make it a duplicate. The answers here are much more valuable because they offer specific solutions to this problem which the one you marked a duplicate does not. By our definition, almost every single question asked today should be a duplicate of something because when you get to the root of all the issues today, there are probably very few actual new issues that haven't somewhere been discussed before. – jfriend00 May 30 '14 at 08:45
  • On top of all this, the question you picked as a duplicate is about passing arguments to a function which has NOTHING to do with the solution to this question - this question involves a mistaken understanding of assigning, not passing arguments to functions. I'm voting to reopen. – jfriend00 May 30 '14 at 08:49

4 Answers4

1

This is because clear sets the value of var list, not the .list on the object returned from ListManager(). You can use this instead:

function ListManager() {
    var list = [],
        add = function (element) {
            this.list.push(element);
        },
        clear = function () {
            this.list = [];
        };

    return {
        add: add,
        clear: clear,
        list : list
    };
}
univerio
  • 19,548
  • 3
  • 66
  • 68
  • What's the point of having `var list = []` at all in this scenario? You should just have `list: []` in the returned object and not have the local variable at all. Personally, I dislike this whole design pattern. Since everything is intended to be public here, why not just assign all methods like `this.add = function() {}`? It's way more straightforward. – jfriend00 May 29 '14 at 23:42
  • @jfriend00 Sure, and while we're at it we can inline `var add` and `var list` into the object as well. I was simply illustrating the key changes OP needed to make to make it work. – univerio May 29 '14 at 23:44
  • OK, fair enough. I added my own answer that offers the simpler version of code. – jfriend00 May 29 '14 at 23:54
0

Using your current structure, you could do:

function ListManager() {
    var list = [],
        add = function (element) {
            list.push(element);
        },
        clear = function () {
            list = [];
        };
        getList=function(){
            return list;
        }


    return {
        add: add,
        clear: clear,
        list : list,
        getList: getList
    };
};

var manager = new ListManager();
manager.add("something");
console.log(manager.getList()); // ["something"]
manager.clear();
console.log(manager.getList()); // []
juvian
  • 15,875
  • 2
  • 37
  • 38
0
    function ListManager() {
    var list = [],
        add = function (element) {
            this.list.push(element);
        },
        clear = function () {
            this.list = [];
        };

    return {
        add: add,
        clear: clear,
        list : list
    };
};

var manager = new ListManager();
manager.add("something");
manager.clear();
console.log(manager.list.length); // <= now outputs "0"!
amunozdv
  • 111
  • 3
0

As has already been explained, your issue is that when you do list = [], you are changing the local variable list, but you aren't changing this.list as they are two separate variables. They initially refer to the same array so if you modified the array rather than assigning a new one to just one of the variables, they would both see the change.

Personally, I think you're using the wrong design pattern for creating this object that just makes things more complicated and makes it more likely you will create problems like you did. That design pattern can be useful if you want to maintain private instance variables that are not accessible to the outside world, but it creates a more complicated definition and maintenance if everything is intended to be public.

One of my programming goals is to use the simplest, cleanest way of expressing the desired functionality.

So that end, since everything in this object is intended to be public and accessible from outside the object, this is a whole lot simpler and not subject to any of the types of problems you just had:

function ListManager() {
    this.list = [];
    this.add = function(element) {
       this.list.push(element);
    }
    this.clear = function() {
        this.list = [];
    }    
}

Or, perhaps even use the prototype:

function ListManager() {
    this.list = [];
}

ListManager.prototype = {
    add: function(element) {
       this.list.push(element);
    },
    clear: function() {
        this.list = [];
    }
};
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • You are right, this design pattern is definitely not the best for declaring public-only-properties. But the posted code is just a simplified version of my real object where I handle private properties as well. :-) – Michael Hilus May 30 '14 at 08:01