Erroneous Behavior Of Local Variables In Closures

I am stuck at the following code. At first I'll describe the use-case: The function 'addPreset' gets called with an instance of ColorGradient. When calling this.listController.addI

Solution 1:

Answer can be found in this question: Doesn't JavaScript support closures with local variables?

Solution 2:

Someone please correct me if I am wrong, as I am still fairly new to JavaScript closures and scope. But it would seem to me that the wrapping anonymous function you have is simply there to provide a proper scoped variable/closure for the function it is returning. Could this be simplified as such?

functionaddPreset(cg) {
            if (!(cg instanceofColorGradient)) {
                thrownewTypeError("PresetManager: Cannot add preset; invalid arguments received");
            }

            var closured = cg;
            var newIndex = this.listController.addItem(cg.getName(), {
                onSelect: function() {
                    // addPreset's scope should now be broken
                    GLab.ColorSlider.applyColorGradient(closured);
                    console.log(closured);
                }
            });

            this.presetList[newIndex] = cg;
        }

Solution 3:

Just want to tell you, that I finally solved my problem by myself. It cost me almost 2 days (in the sparetime) to puzzling it out, but I think its worth that. At least my code remained elegant and I definitely got the whole thing with closures. Let's have a look:

My faulty code

Part 1 of 2:

functionaddPreset(cg) {
    if (!(cg instanceofColorGradient)) {
        thrownewTypeError("PresetManager: blablabla");
    }
    // calls the function in Part 2var newIndex = this.listController.addItem(cg.getName(), {
        onSelect: (function(cg2) {
            returnfunction() {
                // addPreset's scope should now be broken
                GLab.ColorSlider.applyColorGradient(cg2);
                console.log(cg2);
            }
        })(cg)
    });

    this.presetList[newIndex] = cg;
}

Part 2 of 2:

// The method being called by this.listController.addItem(..)
function addItem(caption, args) {
    var _this = this,
        currIndex,
        id,
        newItem
        itemSelectCb = (!!args && typeof args.onSelect == "function") ?
                            args.onSelect :
                            undefined;

    currIndex = this.numOfItems;
    id = this.ITEM_ID_PREFIX + currIndex;
    newItem = this.$itemTemplate
        .clone()
        .text(caption)
        .attr("id", id)
        .bind("click", function(e) {
            e.stopPropagation();
            if (typeof itemSelectCb != "undefined") {
                itemSelectCb();
            }    
            _this._onSelect($(".ListWrapperItem").index(this));
        })
        .appendTo(this.$container);

    this.numOfItems = $("." + this.DEFAULT_ITEM_CLASS, this.$container).length;

    return currIndex;
}

The fixed code

The bug was in Part 2; when calld jQuery's bind-method for adding an click-event-listener I used an anonymous function (= new closure), but referenced itemSelectCb inside; so the anonymous function's scope stayed "connected" to the one of addItem. Everytime I called addItem, an other value were assigned toitemSelectCb what lead to the unknown sideeffect, that all references to itemSelect inside previously created anonymous functions are pointing to that value. What meant, that the last assigned value, had been used by all anonymous function.

To "break" the scope, all I had to do was to modify the lines of Part 2 where the event-handler for jQuery's bind was created. The fixed code looks then like this:

function addItem(caption, args) {
    var _this = this,
        currIndex,
        id,
        newItem
        itemSelectCb = (!!args && typeof args.onSelect == "function") ?
                            args.onSelect :
                            undefined;

    currIndex = this.numOfItems;
    id = this.ITEM_ID_PREFIX + currIndex;
    newItem = this.$itemTemplate
        .clone()
        .text(caption)
        .attr("id", id)
        .bind("click", (function(itemSelectCb) {    

            return function(e) {                    
                e.stopPropagation();
                if (typeof itemSelectCb != "undefined") {
                    itemSelectCb();
                }    
                _this._onSelect($(".ListWrapperItem").index(this));                    
            }

        })(itemSelectCb))
        .appendTo(this.$container);

    this.numOfItems = $("." + this.DEFAULT_ITEM_CLASS, this.$container).length;

    return currIndex;
}