3

I have the following javascript code for D3, which behaves strange in my opinion. As I'm new to D3 I might be missing something, but I have no idea at all. The problem is marked by the alert calls:

I loop over a set of data sets which are drawn as lines (stripped from the code) and as circles/dots on these lines. When I click on the circle, I want to execute some code in the onClick handler. The onClick handler is a closer defined per loop iteration which is passed to the D3 on() method. Quite normal javascript stuff so far. But:

The first alert always displays the correct value as I would expect it. I would the value of chartDataItem to be bound to the onClick closure, so it's supposed to have the same value when it's called. But if onClick is executed, I always get the value from the last iteration.

Is there something special with the D3 on() method that I don't see?

for(var i=0; i<chartData.length; i++){

    var chartDataItem = chartData[i];
    var currentData = chartDataItem["data"];

    alert(chartDataItem["name"]); // <- displays the correct value per iteration
    var onClick = function(d){
        alert(chartDataItem["name"]); // <- should bind chartDataItem to the scope of the closure?!
    }

    var dataLines = lineChart.dataLinesGroup.selectAll('.data-line color' + i)
            .data([currentData]);

    // Draw the lines
    ...

    // Draw the points
    lineChart.dataCirclesGroup = lineChart.svg.append('svg:g');

    var circles = lineChart.dataCirclesGroup.selectAll('.data-point color' + i)
        .data(currentData);

    circles
        .enter()
            .append('svg:circle')
                .attr('class', 'data-point color' + i)
                .style('opacity', 1e-6)
                ... more attr and style calls ...
                .on("click", onClick)
            .transition()
            .duration(0)
                .attr('cx', function(d) { return x(d.title) })
                .attr('cy', function(d) { return y(d.value) })
                .style('opacity', 1);
    ...
}
Achim
  • 15,415
  • 15
  • 80
  • 144

2 Answers2

3

Creating javascript functions in a loop is a little tricky; on every iteration of the loop chartDataItem is redefined. Since each onClick function looks to chartDataItem to see what to text to alert, the last value of chartDataItem will be alerted each time. You can verify this by running

chartDataItem = 'test alert'

in the console after your loop has run, then clicking on any of the circles.

To get around this, you can use a closure to prevent each function's alert text from being modified:

var onClick = function(alertText){
  return function(){ alert(alertText); };
}(chartDataItem['name']);

Alternatively, using .forEach() instead of a for loop avoids this problem by executing each cycle of the loop in its own function.

An even better solution: http://bost.ocks.org/mike/nest/

Community
  • 1
  • 1
Adam Pearce
  • 9,243
  • 2
  • 38
  • 35
2

The issue has to do with a for loops body not being a closure, so you are redefining stuff each time it loops, and it ends up using the definition of the variables in the last iteration of the loop.

Here's a fiddle on a simplified version of your problem that uses a forEach: http://jsfiddle.net/reblace/GxyK6/

var chartData = [{data: "one"}, {data: "two"}, {data: "three"}];
var body = d3.select("body");

chartData.forEach(function(d){
    var chartDataItem = d;

    var onClick = function(d){
        alert(chartDataItem.data);
    }

    var div = body.append("div");
    var words = div.selectAll("div").data([chartDataItem]);
    words.enter().append("div").text(function(d) { return d.data; } ).on("click", onClick);    
});

With a forEach, the body of the forEach is a closure, so the local scope of the closure is preserved.

reblace
  • 4,115
  • 16
  • 16