JS Assigning Callbacks through Iteration

Question

I have a small program that uses RaphaelJS to accept some nodes and edges, much like arborjs and sigmajs, to produce tree graphs. I would use either of the said libraries but I am trying to learn some JS and graph theory and I figured why not try something like that on my own.

The nodes are iterated through and placed onto the paper, and a callback function is given for hover over and hover out. The animation part of the hover over hover out works however the popup text shows the last value that was apart of the iteration when it was assigned.

I have found that this is because I need to use closures, however I have attempted to write a closure following several different examples that I have found.

I have posted an example on JSFiddle: link

var jgraph = {
obj_nodes : [],
obj_edges : [],
addNodes : function(obj) {
    this.obj_nodes[obj.id] = obj;
},
getNodes : function() {
    return this.obj_nodes;
},
addEdges : function(obj) {
    this.obj_edges[obj.id] = obj;
},
getEdges : function() {
    return this.obj_edges;
},
createGraph : function(paper, obj) {
    var point_x, point_y,
    source_x, source_y, target_x, target_y, popup;
    for (var i = 0; i < obj.nodes.length; i++) {
        this.obj_nodes[obj.nodes[i].id] = obj.nodes[i],
        point_x = this.obj_nodes[obj.nodes[i].id].x, 
        point_y = this.obj_nodes[obj.nodes[i].id].y;
        popup = jgraph.createPopup(paper, obj.nodes[i].id);
        paper.circle(point_x, point_y, 10).attr({
                fill : "#e74c3c",
                stroke : "none"
            }).hover(function() {
                this.animate({
                    fill : "#3498db",
                    transform : "s1.5"
                }, 900, "elastic");
                popup.show().toFront();
            }, function() {
                this.animate({
                    fill : "#e74c3c",
                    transform : "s1"
                }, 900, "elastic");
                popup.hide();
        });
    }
    for (var i = 0; i < obj.edges.length; i++) {
        this.obj_edges[obj.edges[i].id] = obj.edges[i];
        source_x = this.obj_nodes[obj.edges[i].source].x,
        source_y = this.obj_nodes[obj.edges[i].source].y,
        target_x = this.obj_nodes[obj.edges[i].target].x,
        target_y = this.obj_nodes[obj.edges[i].target].y;
        p.path("M " + source_x + " " + source_y + " L " + target_x + " " + target_y).toBack().attr({
            stroke : "#e74c3c",
            "stroke-width" : 4
        });
    }
},
createPopup : function(paper, id) {
    return paper.text(this.obj_nodes[id].x, this.obj_nodes[id].y - 20, this.obj_nodes[id].label).hide().attr({
        "font-size" : "13px"
    });
}
};

Thanks in advance!

Answer 1

Closures can certainly be maddening. In this case, the problem is that you're installing your hover handlers in the context of the for loop, and the closure ties the handler's reference to popup to the variable defined outside of the context of the for loop. Naturally, by the time the hover handlers fire, the for loop has finished executing and the value of popup reflects the last label the loop handled.

Try this instead -- you're basically just declaring an anonymous function inside your for loop and passing each value of popup into that function, which makes the closure bind to the values of specific calls to that anonymous function:

        popup = jgraph.createPopup(paper, obj.nodes[i].id);
        ( function( popup )
          {              
            paper.circle(point_x, point_y, 10).attr({
                    fill : "#e74c3c",
                    stroke : "none"
                }).hover(function() {
                    this.animate({
                        fill : "#3498db",
                        transform : "s1.5"
                    }, 900, "elastic");
                    popup.show().toFront();
                }, function() {
                    this.animate({
                        fill : "#e74c3c",
                        transform : "s1"
                    }, 900, "elastic");
                    popup.hide();
            });
          } )( popup );

If I were you, I would probably assign the label to each element using Raphael elements' data method, and then create and destroy the text elements each time a hover is started and ended respectively. That way you don't need to worry about managing many invisible elements at all =)

Here's a fork of your fiddle in working condition.