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.