JSON to data structure in Javascript

Question

I've never formally learned javascript, and I feel very much like I'm unaware of 'good' practice.

I'm working on this open source project, and in particular the code that converts the incoming JSON into a usable data structure in this file

The relevent function looks like this:

   function start() {
       key = "toppage";
       utterances = {};
       links = {};
       colours = {};
       icons = {};
       labels = {};
       slide_number = {};
       var req = new XMLHttpRequest();
       req.open("GET", "pageset.json");
       req.overrideMimeType("application/json");
       req.send(null);
       req.onreadystatechange = function() {
           if (req.readyState == 4 && req.status == 200) {
               var obj = JSON.parse(req.responseText);
               for (grid in obj.Grid) {
           console.log(obj.Grid[grid][0])
                   labels[obj.Grid[grid][0]] = obj.Grid[grid][1];
                   utterances[obj.Grid[grid][0]] = obj.Grid[grid][2];
                   links[obj.Grid[grid][0]] = obj.Grid[grid][3];
                   icons[obj.Grid[grid][0]] = obj.Grid[grid][4];
                   colours[obj.Grid[grid][0]] = obj.Grid[grid][5];
                   slide_number[obj.Grid[grid][0]] = obj.Grid[grid][6];
           if(obj.Grid[grid][6]==0){
            key=obj.Grid[grid][0]
            }
               }
               grid_size_rows = obj.Settings[0];
               grid_size_columns = obj.Settings[0];
               setup_messagewindow();
               setup_table();
               load_page(key);
           }
       };
       //TODO - needs an error message if the JSON doesn't load
   }

I have NO idea if this is a reasonable way to populate structures in javascript. It works, but I feel like it could be a hell of a lot more elegant. Any comments people have would be welcome.

Answer 1

First of all,you declare them without anything,so it's probably declared globally,this is not good,with this amount of variables in global.

Second of all,ES6 Destructuring assignment

var arr=[1,2,3,4,5];
var [a,b,c,d,e]=arr;
console.log(a,b,c,d,e); // 1,2,3,4,5

Using the same way,

for (grid in obj.Grid) {
    var i=obj.Grid[grid][0];
    var [unused,labels[i], utterances[i], links[i], icons[i], colours[i], slide_number[i]]
    =obj.Grid[grid]
    //....
}

Answer 2

• Declare variables with let (if it gets reassigned) or const (if it never gets reassigned).
  I would recommend against using var nowadays. There are no use cases I know of where it is better than let.

  As far as I can see, all variables in your posted code can be declared const.

• Do not use for (const entry in obj) pattern for iterating keys. You use it here: for (grid in obj.Grid).

  Modern alternatives:

  for (const [key, value] of Object.entries(obj)) {
    // ...
  }
  
  for (const key of Object.keys(obj)) {
    const value = obj[key]
    // ...
  }
  

• Use === for comparison, e.g. req.status === 200

• You could use the newer Fetch API as a replacement for XMLHttpRequest. In my opinion, this is worthwhile just for the Promise-driven code you get for free. For example, your //TODO - needs an error message if the JSON doesn't load can be expressed as .catch(err => /* error handling */) (note that fat arrow function syntax).

• Extract "pageset.json" into a constant: const ASSET_JSON_ENDPOINT = "pageset.json" and put it at the beginning of the function or even outside of the function in the outer scope of your

  • file or
  • module (ES6+ terminology) or
  • your Immediately-Invoked Function Expression (IIFE)

  
  Generally, I would recommend using modules (with import ... from ... and export ... syntax). They supersede IIFEs.

  Also choose a better name than "asset" depending on the kind of data you're loading.

• Properly indent/auto-format your code.