1

I'm new to js and have read that there are "monsters lurking in the shadows" when it comes to the use of variables and their respective scope and how hidden data is.

I'm using d3.js and loading data in from a csv but I only want to make a GET call once as more complex visualisations may involve 10 csv files some containing upto 10,000 rows of data - therefore I've added a variable at the top of my script "var data;" which gets filled on initial rendering then the variable gets used on susequent interations from the user.

Is my approach safe? If there is a better pattern I should use what is it?

BarData.csv looks like this:

Fruit,dt,amount
Apple,12/28/2016,-1256
Apple,12/29/2016,-500
Apple,12/30/2016,3694
Apple,12/31/2016,5586
Apple,1/1/2017,4558
Apple,1/2/2017,6696
Apple,1/3/2017,7757
Apple,1/4/2017,8528
Apple,1/5/2017,5543
Apple,1/6/2017,3363
Apple,1/7/2017,5464
Pear,12/25/2017,250
Pear,12/26/2017,669
Pear,12/27/2017,441
Pear,12/28/2017,159
Pear,12/29/2017,357
Pear,12/30/2017,775
Pear,12/31/2017,669

The code is all in a single html file as follows:

<!DOCTYPE html>
<html>

<head>
  <meta http-equiv="Content-type" content="text/html; charset=utf-8">
  <title>BAR SINGLE FUNCTION</title>
  <script src="http://d3js.org/d3.v3.js"></script>
  <style type="text/css">
    #radioDiv {
      top: 45px;
      font-family: verdana;
      font-size: 8px;
      width: 455px;
    }

    #TOPbarChart {
      position: absolute;
      top: 50px;
      left: 30px;
      width: 750px;
      height: 195px;
    }

    .axis--y path,
    .axis--x path {
      display: none;
    }

    .axis--x line,
    .axis--y line {
      stroke: black;
      fill: none;
      stroke-width: 2px
    }

    .yAxis text,
    .xAxis text {
      font: 7pt Verdana;
      stroke: none;
      fill: black;
    }

    .title,
    .titleX {
      font-family: Verdana;
      font-size: 10px;
    }
  </style>
</head>

<body>
  <div id="radioDiv">
    <label>
      <input id="radioFrt" type="radio" name="frt" value="Apple" class="radioB" checked> APPLE
    </label>
    <label>
      <input type="radio" name="frt" value="Pear" class="radioB"> PEAR
    </label>
  </div>
  <div id="TOPbarChart"></div>
  <script type="text/javascript">
    var currentFruit = "Apple";
    var currentColr = "#00a5b6";

    var barDataCSV_Dly = "BarData.csv";

    var data;
    //
    //
    // radio button
    document.getElementById("radioFrt").checked = true;
    d3.selectAll('input[name="frt"]').on("change", function change() {
      currentFruit = this.value;
      //load();
      TOPbarChart(currentFruit, currentColr);
    });

    //FORMATS 
    var parseDate = d3.time.format("%m/%d/%Y").parse;



    // 
    // BASIC SIZING
    // 
    function barChartBasics() {
      var margin = {
          top: 25,
          right: 35,
          bottom: 25,
          left: 70
        },
        width = 550 - margin.left - margin.right,
        height = 155 - margin.top - margin.bottom,
        colorBar = d3.scale.category20(),
        barPaddingFine = 1,
        barPaddingThick = 2;
      return {
        margin: margin,
        width: width,
        height: height,
        colorBar: colorBar,
        barPaddingFine: barPaddingFine,
        barPaddingThick: barPaddingThick
      };
    }


    // create svg element
    var basics = barChartBasics();
    var svg = d3.select("#TOPbarChart")
      .append("svg")
      .attr({
        "width": basics.width + basics.margin.left + basics.margin.right,
        "height": basics.height + basics.margin.top + basics.margin.bottom,
        id: "svgTOPbarChart"
      });

    // create svg  group
    var plot = svg
      .append("g")
      .attr({
        "transform": "translate(" + basics.margin.left + "," + basics.margin.top + ")",
        id: "svgPlotTOPbarChart"
      });

    var axisPadding = 2;
    var leftAxisGroup = svg
      .append('g')
      .attr({
        transform: 'translate(' + (basics.margin.left - axisPadding) + ',' + (basics.margin.top) + ')',
        'class': "yAxis axis--y",
        id: "yAxisGTOPbarChart"
      });

    var bottomAxisGroup = svg
      .append('g')
      .attr({
        'class': "xAxis axis--x",
        id: "xAxisGTOPbarChart"
      });

    var titleTxt = svg.append("text")
      .attr({
        x: basics.margin.left + 12,
        y: 20,
        'class': "title",
        'text-anchor': "start"
      })

    // create scales with ranges
    var xScale = d3.time.scale().range([0, basics.width]);
    var yScale = d3.scale.linear().range([basics.height, 0]);




   function load(){
     d3.csv(barDataCSV_Dly, function(rows) {
        data = rows;
        TOPbarChart(currentFruit, currentColr)
     })
   }


    function TOPbarChart(
      frt, colorChosen) {

      // get the data
      //d3.csv(barDataCSV_Dly, function(rows) {

        TOPbarData = data.map(function(d) {
          return {
            "Fruit": d.Fruit,
            "dt": parseDate(d.dt),
            "amount": +d.amount
          };
        }).filter(function(row) {
          if (row['Fruit'] == frt) {
            return true;
          }
        });


        // create domains for the scales
        xScale.domain(d3.extent(TOPbarData, function(d) {
          return d.dt;
        }));
        var amounts = TOPbarData.map(function(d) {
          return d.amount;
        });
        var yMax = d3.max(amounts);
        var yMin = d3.min(amounts);
        var yMinFinal = 0;
        if (yMin < 0) {
          yMinFinal = yMin;
        }



        yScale.domain([yMinFinal, yMax]);


        // introduce the bars
        // var plot = d3.select("#svgPlotTOPbarChart")
        var sel = plot.selectAll("rect")
          .data(TOPbarData);

        sel.enter()
          .append("rect")
          .attr({
            x: function(d, i) {
              return xScale(d.dt);
            },
            y: function(d) {
              return yScale(Math.max(0, d.amount));
            },
            width: (basics.width / TOPbarData.length - basics.barPaddingFine),
            height: function(d) {
              return Math.abs(yScale(d.amount) - yScale(0));
            },
            //fill: colorChosen,
            fill: function(d, i) {
              return d.amount < 0 ? "#FF0000" : colorChosen;
            },
            // fill: function(d, i) {
            //            var col = colorChosen
            //              if (d.amount < 0) {
            //                col = "#FF0000";
            //            }
            //            return col;
            //        },
            'class': "bar"
          });

        // this little function will create a small ripple affect during transition
        var dlyRipple = function(d, i) {
          return i * 100;
        };

        sel
          .transition()
          .duration(dlyRipple) //1000
          .attr({
            x: function(d, i) {
              return xScale(d.dt);
            },
            y: function(d) {
              return yScale(Math.max(0, d.amount));
            },
            width: (basics.width / TOPbarData.length - basics.barPaddingFine),
            height: function(d) {
              return Math.abs(yScale(d.amount) - yScale(0));
            },
            //fill: colorChosen
            fill: function(d, i) {
              var col = colorChosen
              if (d.amount < 0) {
                col = "#FF0000";
              }
              return col;
            },
          });

        sel.exit().remove();


        // add/transition y axis - with ticks and tick markers
        var axisY = d3.svg.axis()
          .orient('left')
          .scale(yScale)
          .tickFormat(d3.format("s")) // use abbreviations, e.g. 5M for 5 Million
          .outerTickSize(0);
        leftAxisGroup.transition().duration(1000).call(axisY);

        // add/transition x axis - with ticks and tick markers
        var axisX = d3.svg.axis()
          .orient('bottom')
          .scale(xScale);

        bottomAxisGroup
          .attr({
            transform: 'translate(' + (basics.margin.left + ((basics.width / TOPbarData.length) / 2)) + ',' + (basics.margin.top + basics.height) + ')',
          })
          .transition().duration(1000).call(axisX.ticks(5));


        titleTxt.text("Daily: last " + TOPbarData.length + " days");
        // console.log(TOPbarData.length)

      //});
    }

    //
    //
    //
    //
    load()
    //TOPbarChart(currentFruit, currentColr);
    //
    //
    //
    //
  </script>
</body>

</html>

Working example here on Plunker:

http://plnkr.co/edit/lfWgibRsm6mMzLYSgR2o?p=info


Edit

I tried Piotr's suggestion so I now have this:

var Module_BarDataDaily = (function() {
    var data;
    d3.csv("myData.csv", function(rows) {
        data = rows;
    });
    return {
        dataX: data
    }
})();

Then in a subsequent function I have the following:

var x = Module_BarDataDaily.dataX;

TOPbarData = x.map(function(d) {  /<<< Type error x is undefined
    return {
...

It is throwing an undefined error at the line marked above.

whytheq
  • 34,466
  • 65
  • 172
  • 267
  • It's unclear to me what you're asking in the body of the question. The answer to the title is "no, it's not best practice to have a global `data` -- or indeed, any other global." – T.J. Crowder Mar 04 '17 at 16:42
  • @T.J.Crowder apologies for lack of clarity. In terms of this "Is my approach safe? If there is a better pattern I should use what is it?" you are saying that my approach is not safe ... so what appraoch is best practise? – whytheq Mar 04 '17 at 17:53

2 Answers2

1

Caching records in variable is not bad until it's private and not visible anywhere else in scripts.

Your current approach sets data variable in global namespace, essentially if any other plugin you include will also have variable called data, might override it's value. - That is not safe.

To hide variable from rest of the world, you would want to include all your code in self-invoking function:

(function() {
  var data = ...; // It's accessible only in that function

  // Your JavaScript code

})();

Related notes:


If you want to share variable between files, do it within object (called module), e.g.:

var Module = (function() {
  var data = ...
  // Your JavaScript code

  return {
     data : data
  }      
})();

// Now it's accessible with Module.data

JavaScript provides build-in module system, that is similar to Python. However it's not yet supported in browsers.

Related notes:


PS: Separate your CSS and JavaScript to external files to avoid mess.

Community
  • 1
  • 1
Piotr Lewandowski
  • 6,540
  • 2
  • 27
  • 32
1

After update your question, there is another problem called asynchronous data.

Library function d3.csv(...) works asynchronous from other calculations, which means you can't never be sure that data is filled. It might be immediately, or it might be after few seconds.

That's why you get x is undefined, because in moment of parsing that code, d3.csv didn't finished calculations yet.

The easiest attempt to handle this code is to use callback function. Basically, your module returns function loadData that accepts function which will be invoked only when module will know that data is filled.

var Module_BarDataDaily = (function() {
    var data;

    return {
        loadData: lazyLoadData
    };

    function lazyLoadData(callback) {
        if (data) {
            callback(data); // data already exists, run function
        } else {
            d3.csv("myData.csv", function(rows) {
                data = rows;
                callback(data); // data is filled, run function
            });
        }
    }
})();

Here is how it can be used with that attempt.

Module_BarDataDaily.loadData(function(data) {
    // this function loads only after data is filled
    TopChartData = data.map(...);
});

What is callback?

In plain English, a callback function is like a Worker who "calls back" to his Manager when he has completed a Task.


Note: There are many other, potentially better attempts to solve such problems like Promise from ES2015, await that will be released with ES2017.

Community
  • 1
  • 1
Piotr Lewandowski
  • 6,540
  • 2
  • 27
  • 32
  • this is very kind of you Piotr to spend the time to add this additional information for me - thank you. – whytheq Mar 06 '17 at 19:05