CODEWITHSUNDEEP
javascriptsvgd3.js
Hanfei Sun
Hanfei Sun

Reputation: 47051

What is the recommended way to refactor code written with d3.js like this?

Here is the code.

var mapXOffset = 20;
var mapYOffset = 20;
var personSize = 4;
var redCount = 200;
var blueCount = 200;

function redraw() {
    var svg = d3.select("svg");
    var tempArray = makeRandomArray(0,10000, redCount+blueCount);
    var redArray = tempArray.slice(0,redCount);
    var blueArray = tempArray.slice(redCount,redCount+blueCount);

    var redData = svg.selectAll("rect.red")
        .data(redArray);
    var blueData = svg.selectAll("rect.blue")
        .data(blueArray);


    redData.enter().append("rect")
    .attr("x", function(d, i)
          {
          return mapXOffset + personSize * (d%100);
          })
    .attr("y", function(d)
          { 
          return mapYOffset + personSize * (d/100);
          })
    .attr("width", personSize)
    .attr("height", personSize)
    .attr("class", "red");

    blueData.enter().append("rect")
    .attr("x", function(d, i)
          {
          return mapXOffset + personSize * (d%100);
          })
    .attr("y", function(d)
          { 
          return mapYOffset + personSize * (d/100);
          })
    .attr("width", personSize)
    .attr("height", personSize)
    .attr("class", "blue");

    redData.transition()
    .duration(1000)
    .attr("x", function(d, i)
          {
          return mapXOffset + personSize * (d%100);
          })
    .attr("y", function(d)
          { 
          return mapYOffset + personSize * (d/100);
          });    

    blueData.transition()
    .duration(1000)
    .attr("x", function(d, i)
          {
          return mapXOffset + personSize * (d%100);
          })
    .attr("y", function(d)
          { 
          return mapYOffset + personSize * (d/100);
          });    

};

As is shown, redData and blueData do things almost the same except for the .attr("class", ""); part. Which way is better to refactor the code: (1) wrap the redundent codes into a function (2) make an array [redArray, BlueArray] and iterate on it . Or other way preferrd?

Upvotes: 0

Views: 417

Answers (2)

t.888
t.888

Reputation: 3902

It looks like you could refactor simply by creating an updateData() method and call it from redraw.

function updateData(selector, class, array) {

  var svg = d3.select("svg");
  var data = svg.selectAll(selector).data(array);

  data.enter().append('rect')
    .attr('x', function(d, i) {
      return mapXOffset + personSize * (d%100); })
    .attr('y', function(d) {
      return mapYOffset + personSize * (d/100); })
    .attr("width", personSize)
    .attr("height", personSize)
    .attr("class", class);

  data.transition()
    .duration(1000)
    .attr("x", function(d, i) {
      return mapXOffset + personSize * (d%100); })
    .attr("y", function(d) { 
      return mapYOffset + personSize * (d/100); });    
}


function redraw() {
    var tempArray = makeRandomArray(0,10000, redCount+blueCount);
    var redArray = tempArray.slice(0,redCount);
    var blueArray = tempArray.slice(redCount,redCount+blueCount);

    updataData('rect.red', 'red', redArray);
    updataData('rect.blue', 'blue', blueArray);
};

Alternately you could define svg in refresh and pass it in as another parameter. If you add too many more parameters, consider passing them in as an object literal.

You could also iterate on an array, but it would require more parameters per array element than just redArray and blueArray. Ultimately, it's a judgment call based on which method produces the most concise and/or readable code.

Here's how I imagine the array version would look like. It has a certain appeal in that it doesn't require a separate function definition.

function redraw() {
  var tempArray = makeRandomArray(0,10000, redCount+blueCount);
  var redArray = tempArray.slice(0,redCount);
  var blueArray = tempArray.slice(redCount,redCount+blueCount);

  var svg = d3.select("svg");

  var red = {selector: 'rect.red', class: 'red', array: redArray};
  var blue = {selector: 'rect.blue', class: 'blue', array: blueArray};

  [red, blue].forEach(function(item) {
    var data = svg.selectAll(item.selector).data(item.array);
    data.enter().append('rect')
      .attr('x', function(d, i) {
        return mapXOffset + personSize * (d%100); })
      .attr('y', function(d) {
        return mapYOffset + personSize * (d/100); })
      .attr("width", personSize)
      .attr("height", personSize)
      .attr("class", item.class);

    data.transition()
      .duration(1000)
      .attr("x", function(d, i) {
        return mapXOffset + personSize * (d%100); })
      .attr("y", function(d) { 
        return mapYOffset + personSize * (d/100); });    
  });
}

Upvotes: 1

ninjaPixel
ninjaPixel

Reputation: 6382

Mike Bostock has written a great article on reusable charts. There is also a very good book called Developing a D3.js Edge, which I highly recommend (it's short and an easy read).

In essence you want to implement charts as closures with getter-setter methods, this enables you to reuse your chart code as often as you like while also giving you the configurability to modify properties of existing chart objects.

Upvotes: 1

Related Questions