0

I have a function within an object constructor that is altering all objects created by that constructor. I'm not sure why. Could someone please take a look at my code and tell me what I'm missing?

A quick description of what is going on:

Warning! It might be easier to just read through the code than try to make sense of my description

I have created two new Arrays. The first one called foos, which will be an array of foo objects each one containing an array of bar objects. The second is called bars which is an array of all bar objects that are available to be added to the foos.foo.bars arrays.

When a new foo object is created using the foo object constructor it is given two arguments(aBars,bBars). aBars is an array of all bar objects to be included in the foo object. bBars is an array of all included bar objects that are considered 'special' in some way. Within the constructor there is a function that runs through every object in the bars array and if it's name value matches that of a string in the aBars argument then it is added to foo.bars array. If it's name value matches a string in the bBars argument it then has it's property bBar set to true, otherwise it's set to false.

The issue I'm having is that on the second foo object constructor when a bar object has bBar set to true or false it also changes that value in that object in all other foo.bars objects.

I realize that this is probably hard to follow. Sorry about that, it's the end of the day.

Found my own answer!

I just realized what the issue is. foos[0].bars[4] and foos[1].bars[3] are not separate objects, they are simply two different variables pointing to the same object. So when one is changed the change shows up on both. Wow, I can't believe I just spent so much time working on this when the answer was a basic fact about how javascript works that I learned back when I first started.

Ok, the new question:

How can I change this code to create duplicates of the objects instead of just pointing at the originals? This is not something I've ever had to do before.

Thanks

jsfiddle

JS:

var foos = new Array();
var bars = new Array();

function foo(aBars,bBars) {
    var $this = this;
    this.aBars = aBars;
    this.bars = new Array();
    bars.forEach(function(e,i) {
        if ($this.aBars.lastIndexOf(e.barName) > -1) {
            $this.bars.push(e);
            if (bBars.lastIndexOf(e.barName) > -1) {
                $this.bars[$this.bars.length-1].bBar = true;
            } else {
                $this.bars[$this.bars.length-1].bBar = false;
            }
        }
    });
}
function bar(name) {
    this.barName = name;
}

bars.push(new bar('l'));
bars.push(new bar('m'));
bars.push(new bar('n'));
bars.push(new bar('o'));
bars.push(new bar('p'));

foos.push(new foo(['l','m','n','o','p'],['n','p']));
foos.push(new foo(['l','n','o'],['n','o']));

console.log(foos);
hal
  • 4,845
  • 7
  • 34
  • 57
  • There's just an awful lot of things wrong with this code... But a couple of things: Don't use `new Array`, use `[]`. Always capitalize constructs. Convention matters. Whenever you're defining a function in a constructor, think _`prototype`_ and most of all: _read up on `this` and the call-context of the `forEach` callback_ – Elias Van Ootegem May 22 '13 at 21:20
  • Why shouldn't I use `new Array`? I've read, and was taught, that when defining an empty array `new Array` was better. Yeah, I usually capitalize constructs, that was more of a typo in this due to trying to recreate the issue quickly in a jsfiddle. How was I not 'thinking prototype'? – hal May 22 '13 at 23:00
  • Where were you taught `new Array` is better? The array constructor [is slower](http://stackoverflow.com/questions/7375120/why-is-arr-faster-than-arr-new-array), it's badly overloaded (`new Array(10)` vs `new Array('10')`) and you were not thinking _prototype_ when you wrote `bars.foreach(function`, which isn't the same as `this.bars`, and which constructs a foreach callback for each instance you create. Best make that into a prototype method... – Elias Van Ootegem May 23 '13 at 09:09
  • Thanks for clarifying. I will start using `[]`. `bars.foreach()` is running the function for each object in the global variable `bars`. `this.bars` would target an empty array. The point of `bars.foreach` callbacks is to act as a filter only adding desired objects from the global variable to this new array. Am I missing your point here? I get your point about using a prototype method though, an overlook on my part for sure. Thanks for the help. I'm still relatively new to this and appreciate guidance wherever I can get it. – hal May 23 '13 at 14:51
  • Well, having the behaviour of a constructor (or any function, for that matter) depend on a _global_ variable isn't what I'd call a great idea. Especially if that global variable has the same name as a property. If I were to see this code at work, I'd assume someone made a mistake and shout at him for using globals. I suggest you look into [closures](http://stackoverflow.com/questions/16472577/javascript-how-to-make-this-code-work/16472719#16472719), so you can _"bind"_ the `bars` array to the constructor. – Elias Van Ootegem May 23 '13 at 19:30

1 Answers1

0

The only way to achieve that would be to replace this line

$this.bars.push(e);

in your 'foo'-constructor with this one:

$this.bars.push(new bar(e.barName));

Cloning objects in javascript is only possible by copying their properties.

nepumuk
  • 86
  • 6