Using array values to create variables, if statements and highchart dataset

I am trying to make my script maintenance-free by replacing some "static" variables and labels with dynamic ones. The main function of my code is to extract data from a SharePoint-list which contains a number of list items with several columns (including a column for school), then count the number of items based on a set criteria (e.g. how many from the school "BI") and then use the data to create a pie chart.

Currently, I have manually made up variables, if statements, and highcharts data labels. However, the list of schools is stored in another SharePoint-list which I can extract in JSON format like this: ["(Ikke oppgitt)", "BI", "CBS", "HiOA", "HiST", "NHH", "NMBU", "NTNU", "UiA", "UiO", "Øvrige"] . The "school"-column in the main SharePoint-list is simply a lookup-column from the school-list.

I am trying to make this smarter by first extracting the schools from the SP-list, then create a variable for each school with the value = 0, then make an if statement for each school that counts the number of occurrences of the school, and then push the school name and the corresponding value from the variable to the highchart dataset to create a pie chart. Is there any way to do this? I am trying to do this so that the code does not need to be changed in case a new school is added to the school SP-list.



`$.when(queryData).done(function() {
antallAnsatteBI = 0
antallAnsatteCBS = 0
antallAnsatteHiOA = 0
antallAnsatteHiST = 0
antallAnsatteNHH = 0
antallAnsatteNMBU = 0
antallAnsatteNTNU = 0
antallAnsatteUiA = 0
antallAnsatteUiO = 0
antallAnsatteØvrigeSkoler = 0

dataqueryData = this.data;
var datalength = dataqueryData.length;
for (var i = 0; i

First, and I say this as a fellow Scandinavian: Just stick to English in your code. It's just easier. As long as you've got words like while, if, throw etc., mixing languages is just strange.

It's even stranger when you're mixing languages within the same word, like createSkolerPie. (Funnily enough, your one comment is in English, while the code is mixed.)

Sure, you can code in any language if you want, but keeping things in English helps readability. You wouldn't unnecessarily mix languages in an essay or letter, would you? So why mix languages in code?

Besides, you may not be the only person to ever work on the code (for instance, here I am, reviewing it! I just happen to understand it, but that's pure chance), and English is the lingua franca among programmers.

As for the code itself:

You're creating a lot of global variables. All your antallAnsatte ("numberOfEmployees", I believe) variables are global, because you didn't properly declare them with the var keyword. This is really bad practice.

I know you want to use the variables in createSkolerPie, but just pass them to the function then. It's what function arguments are for. And it doesn't explain why dataqueryData is global; that one isn't used elsewhere.

dataqueryData is a bit of an odd duck for another reason too: There's no need for it. You're not giving the value a more descriptive name, and it's in fact shorter to write this.data.

You're also missing a few semi-colons, and you've got an extra } on line 50.

Also, if you ever need to parse a bunch of mutually exclusive conditions, don't use if.. else if.. else if..: Use a switch statement.

Anyway, you say you get a list of schools, and then... you write that same list again as a bunch of variables? That can't be right. You say you want it to be more maintainable, yet you hardcode every single school name into the code. And repeat them over and over. This is not very maintainable. It's error prone, and it's just needlessly laborious and repetitive.

Now, I don't see that school list anywhere in your code, but you gave an example of its contents in your question, so I'll just use that in the code below.

Anyway, first order of business: Create an object (aka a hash, aka a dictionary, aka an associative array, aka a map) using the school names as keys, and set zero for the values:

// I don't know how you get this from the server, so just pasted it in here.
// In fact, you could just hard-code it like this and still make the rest
// of the code simpler
var schools = ["(Ikke oppgitt)", "BI", "CBS", "HiOA", "HiST", "NHH", "NMBU", "NTNU", "UiA", "UiO", "Øvrige"];

var employeeCounts = {}; // empty object

// create key/value pairs for each school
for(var i = 0, l = schools.length ; i < l ; i++) {
  employeeCounts[schools[i]] = 0;
}

Or, if you want to be fancy and use reduce you can do:

var employeeCounts = schools.reduce(function (memo, name) {
  memo[name] = 0;
  return memo;
}, {});

In either case, you end up with an object with all the school names as keys, and zeros for the counts.

Now, when you parse the other data, you can do this:

var NOT_GIVEN = '(Ikke oppgitt)'; // stick this in a "constant" to avoid repeating it

for(var i = 0, l = this.data.length ; i < l ; i++) {
  var schoolName = this.data[i].Master.lookupValue;

  if(schoolName === NOT_GIVEN) {
    schoolName = this.data[i].Bachelor.lookupValue;
  }

  // you could write the above as a ternary, but it's not my style.
  // But you want to, it could look like this (or be on one long line):
  //
  // var schoolName = this.data[i].Master.lookupValue !== NOT_GIVEN
  //                  ? this.data[i].Master.lookupValue
  //                  : this.data[i].Bachelor.lookupValue;

  if(schoolName in employeeCounts) { // an extra check, for safety
    employeeCounts[schoolName]++;
  }
}

That's it - that's all the counting done.

Now just convert it to an array highcharts can use:

var chartData = [];
for(var i = 0, l = schools.length ; i < l ; i++) { // using the schools array again to preserve the sorting
  if(schools[i] !== NOT_GIVEN) { // filter out the
    charData.push([schools[i], employeeCounts[schools[i]]]);
  }
}

Or, again, you can use reduce:

var chartData = schools.reduce(function (memo, name) {
  if(name !== NOT_GIVEN) {
    memo.push([name, employeeCounts[name]]);
  }
  return memo;
}, []);

Putting it all together you'd get something like:

```
$.when(queryData).done(function () {
var NOT_GIVEN = '(Ikke oppgitt)';

// again, this shouldn't be hardcoded here - grab it from the server instead
var schools = ["(Ikke oppgitt)", "BI", "CBS", "HiOA", "HiST", "NHH", "NMBU", "NTNU", "UiA", "UiO", "Øvrige"];

var employeeCounts = schools.reduce(function (memo, name) {
memo[name] = 0;
return memo;
}, {});

// let's use forEach here instead, just because
this.data.forEach(function (datum) {
var schoolName = datum.Master.lookupValue

StackExchange Code Review Q#67692, answer score: 3