Cannot make unique javascript object. What's wrong with this code?

Question

I'm trying to make a simple in in-page popup called like this:

var test = new popObject({}); //JSON options

and I'm having trouble because when I create two in a row, and call show() on the first one, the second one always shows. Both are created, but they aren't separate somehow, despite being called with new. What am I doing wrong here? I've included my code, but I have removed out the irrelevant functions for compactness.

function popObject(options) {   
//functions

show = function() {
    console.log(boxselector);
    jQuery(boxselector).css("display", "block");
    return jQuery(boxselector);
}

var hide = function() {...}
var update = function(updateOptions) {...}
var calcTop = function(passedHeight) {...}
var calcLeft = function(passedWidth) {...}
var calcHeight = function(passedHeight) {...}
var stripUnits = function(measure, auto) {...}
var destroy = function() {...}

//public functions

this.show = show;
this.hide = hide;
this.update = update;
this.destroy = destroy;

//constants

name = options.name; //name should never be changed.
boxselector = ".boxcontainer[name=" + options.name + "]";
boxbodyselector = ".boxbody[name=" + options.name + "]";
boxtitleselector = ".boxcontainer[name=" + options.name + "]"
boxboxselector = ".boxbox[name=" + options.name + "]"
title = options.title;
content = options.content;  
width = options.width;
height = options.height;

this.name = name;
this.selectors = [boxselector, boxbodyselector, boxtitleselector, boxboxselector]
this.title = title;
this.content = content;
this.width = width;
this.height = height;

//variables

popupHtml = ...

//init code

jQuery("#dropzone").append(popupHtml); this.init = null;
jQuery(".boxbox[name=" + name + "]").css("top", calcTop(width));
jQuery(".boxbox[name=" + name + "]").css("left", calcLeft(height));
jQuery(".boxbody[name=" + name + "]").css("height", calcHeight(height));
}

Ryan Kinal · Accepted Answer · 2010-08-11 19:20:24Z

5

This is because you're declaring a lot of variables in the global scope. Try the following code instead:

function popObject(options) {   
//functions

this.show = function() {
    console.log(boxselector);
    jQuery(boxselector).css("display", "block");
    return jQuery(boxselector);
}

var hide = function() {...}
var update = function(updateOptions) {...}
var calcTop = function(passedHeight) {...}
var calcLeft = function(passedWidth) {...}
var calcHeight = function(passedHeight) {...}
var stripUnits = function(measure, auto) {...}
var destroy = function() {...}

//public functions

this.show = show;
this.hide = hide;
this.update = update;
this.destroy = destroy;

//constants

var name = options.name; //name should never be changed.
var boxselector = ".boxcontainer[name=" + options.name + "]";
var boxbodyselector = ".boxbody[name=" + options.name + "]";
var boxtitleselector = ".boxcontainer[name=" + options.name + "]"
var boxboxselector = ".boxbox[name=" + options.name + "]"
var title = options.title;
var content = options.content;  
var width = options.width;
var height = options.height;

this.name = name;
this.selectors = [boxselector, boxbodyselector, boxtitleselector, boxboxselector]
this.title = title;
this.content = content;
this.width = width;
this.height = height;

//variables

var popupHtml = ...

//init code

jQuery("#dropzone").append(popupHtml); this.init = null;
jQuery(".boxbox[name=" + name + "]").css("top", calcTop(width));
jQuery(".boxbox[name=" + name + "]").css("left", calcLeft(height));
jQuery(".boxbody[name=" + name + "]").css("height", calcHeight(height));
}

Note all the vars that weren't there before. This defines them as local to the function, and thus local to your object (and also, essentially, private... use this. instead of var to make public members).

Anything that isn't declared with a var or a this. is considered global. So, when you called show(), it used the global show, which referenced the object that was created later.

answered Aug 11, 2010 at 19:20

Ryan Kinal

17.8k7 gold badges48 silver badges63 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Russell Leggett Over a year ago

This is so important to do it cannot be stressed enough. Always use var unless you are absolutely sure you want a global variable, and even then, you probably shouldn't be using a global variable!

Jamie Wong Over a year ago

even if you DO want a global variable, you should probably be doing window.variable_name to make it clear it's not a reassignment of a local variable.

Brian Hicks Over a year ago

Still feel like I'm just getting started with javascript. I had no idea about that. Thanks a bunch.

meder omuraliev · Accepted Answer · 2010-08-11 19:20:44Z

0

What is boxselector? If it's a generic selector then it would select all elements on the page, regardless if its inside of that unique object.

answered Aug 11, 2010 at 19:20

meder omuraliev

187k76 gold badges401 silver badges443 bronze badges

1 Comment

Ryan Kinal Over a year ago

boxselector is declared later in the object. In the original code, it's an implied global. In my solution, it's a local variable.

Jamie Wong · Accepted Answer · 2010-08-11 19:22:36Z

0

When you declare something without var or this within a function definition, such as

boxselector = ".boxcontainer[name=" + options.name + "]";

It creates it in the global namespace (attaches it to window)

Try changing this line to

var boxselector = ".boxcontainer[name=" + options.name + "]";

answered Aug 11, 2010 at 19:22

Jamie Wong

18.4k8 gold badges66 silver badges81 bronze badges

Collectives™ on Stack Overflow

Cannot make unique javascript object. What's wrong with this code?

3 Answers 3

3 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related