# JSON to data structure in Javascript

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 = {};
colours = {};
icons = {};
labels = {};
slide_number = {};
var req = new XMLHttpRequest();
req.open("GET", "pageset.json");
req.overrideMimeType("application/json");
req.send(null);
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];
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();
}
};
//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.

• Could you add a document from the response you're receiving? Also, this is not a complete code. Functions setup_*() are missing. CodeReview does not allow posting partial code with links to GitHub. Dec 27 '17 at 22:09

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]
//....
}

• 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.