HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Create a pop-up class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classcreatepop

Problem

I'm new to OOP, so I tried to learn from tutorials how to create a class in JavaScript/jQuery. But tutorials don't give you feedback from experienced coders. Is the approach to create a pop-up OK (see below)? I know it works (it's tested), but is it prone to errors or does it have redundancies?

var Popup = function (){
    /// PROPERTIES
    var new_id="div"+Date.now();
    var defaults = {
        id: new_id,
        container: "body",
        height: 200,
        width: 200,
        text: " created with \"id\" = "+new_id,
        x: 100,
        y: 100,
        position: "absolute",
        bgcolor: "red"
    };
    this.settings=defaults;
    /// METHODS
    this.create=function (options){
        $.extend(this.settings,options);
        var html= ""+
                this.settings.text+
                "";
        $(this.settings.container).append(html);
        this.update();
    };
    this.update=function (options) {
        $.extend(this.settings,options);
        $("#"+this.settings.id).css({
            "height":this.settings.height+"px",
            "width":this.settings.width+"px",
            "position":this.settings.position,
            "top":this.settings.y,
            "left":this.settings.x,
            "background-color":this.settings.bgcolor
        });
    };
};

/* Popup use:

var pop=new Popup(); // new popup is declared
pop.create({bgcolor:"green"}); // popup appears green instead of read
pop.update({bgcolor:"blue"}); // popup changes to blue

*/


(I know there are a tons of Popup plugins that I could use, but that's irrelevant for my learning goals)

For example, I've realized that this line will cause problems:

this.settings=defaults;


because when "settings" are changed, "defaults" change too (!). Thus, the better alternative is:

this.settings=$.extend(defaults);


or a JavaScript "clone" function like this.

Solution

Most of your code looks good
The one improvement is do not overwrite the initial settings, instead extend from defaults and the options provided. Use $.extend for this

popup.settings = $.extend(defaults, options);


Also store the reference of this into a variable so that it will be used inside event handlers and anonymous callbacks.

var Popup = function() {
    var popup = this,
        new_id = "div" + Date.now(),
        defaults = {
            id: new_id,
            container: "body",
            height: 200,
            width: 200,
            text: " created with \"id\" = " + new_id,
            x: 100,
            y: 100,
            position: "absolute",
            bgcolor: "red"
        };

    popup.create = function(options) {
        popup.settings = $.extend(defaults, options);
        var html = "" +
            popup.settings.text +
            "";
        $(popup.settings.container).append(html);
        popup.update();
    };

    popup.update = function(options) {
        popup.settings = $.extend(defaults, options);
        $("#" + popup.settings.id).css({
            "height": popup.settings.height + "px",
            "width": popup.settings.width + "px",
            "position": popup.settings.position,
            "top": popup.settings.y,
            "left": popup.settings.x,
            "background-color": popup.settings.bgcolor
        });
    };
};

var pop = new Popup(); // new popup is declared

pop.create({
    bgcolor: "green"
}); // popup appears green instead of read

pop.update({
    bgcolor: "blue"
}); // popup changes to blue

Code Snippets

popup.settings = $.extend(defaults, options);
var Popup = function() {
    var popup = this,
        new_id = "div" + Date.now(),
        defaults = {
            id: new_id,
            container: "body",
            height: 200,
            width: 200,
            text: "<div> created with \"id\" = " + new_id,
            x: 100,
            y: 100,
            position: "absolute",
            bgcolor: "red"
        };

    popup.create = function(options) {
        popup.settings = $.extend(defaults, options);
        var html = "<div id=\"" +
            popup.settings.id + "\">" +
            popup.settings.text +
            "</div>";
        $(popup.settings.container).append(html);
        popup.update();
    };

    popup.update = function(options) {
        popup.settings = $.extend(defaults, options);
        $("#" + popup.settings.id).css({
            "height": popup.settings.height + "px",
            "width": popup.settings.width + "px",
            "position": popup.settings.position,
            "top": popup.settings.y,
            "left": popup.settings.x,
            "background-color": popup.settings.bgcolor
        });
    };
};



var pop = new Popup(); // new popup is declared

pop.create({
    bgcolor: "green"
}); // popup appears green instead of read

pop.update({
    bgcolor: "blue"
}); // popup changes to blue

Context

StackExchange Code Review Q#97214, answer score: 2

Revisions (0)

No revisions yet.