5
\$\begingroup\$

So this is what I'm doing:

I have hundreds of arrays, each of them filled with a date in milliseconds in the first position, and the rest of the array is filled with user game scores.

enter image description here

Let's just call this array above var scoresDates

The ultimate goal is to have each of those arrays contain only the date in milliseconds, the smallest score and then the largest score, in that order. To do this, I set up for loop within a for loop, that will loop the larger array and the systematically make several operations until the end result is achieved. I would like to see if there are better ways to do it or optimize this process.

This is what I have...

// Logic that stacks all the scores per unique day from all the user's game scores. All unique dates with subsequent scores are stored in the uniqueDaysScores array.

uniqueDaysScores = [];
uniqueDaysScores.push(scoresDates[0]);

for(var i = 0, ii = scoresDates.length; i < ii; i++){
  var tempArray = [];

  if(scoresDates[i][0] == uniqueDaysScores[0][0]){
    uniqueDaysScores[0].push(scoresDates[i][1]);
  } else{
    tempArray.push(scoresDates[i][0]);
    tempArray.push(scoresDates[i][1]);
    uniqueDaysScores.unshift(tempArray);
  }
}

// Storted scores from smallest to largest for each unique date for user games played. 

scoreSorted = [];

// Logic to take all the scores, sort them from decreasing to increasing order, then package the date, lowest and shortest score into the finished array, scoreSorted.

for(var i = 0, ii=uniqueDaysScores.length; i < ii; i++){
  var scoreSorted1 = [];
  var scoreSortedPrimer = [];

// First level for loop that goes through the unique scores array and ignores the date, while pushing all the scores into the scoreSorted1 array.

  for(var z = 0, zz = uniqueDaysScores[i].length; z < zz; z++){
    if(uniqueDaysScores[i][z] < 1000){
      scoreSorted1.push(uniqueDaysScores[i][z]);
    }
  }

  // Next step is sorting the scoreSorted1 array from lowest score to highest score.
  scoreSorted1.sort(function(a,b){
    return a-b;
  });

  // Find maximum and minimum scores in the array and finally push them into the scoreSortedPrimer array.
  var primerMin = Math.min.apply(Math, scoreSorted1);
  scoreSortedPrimer.push(primerMin);
  var primerMax = Math.max.apply(Math,scoreSorted1);
  scoreSortedPrimer.push(primerMax);

  // Finally, find the date and push it into the scoreSortedPrimer array, so it can be pushed into the main scoreSorted array.

  for(var x = 0, xx = uniqueDaysScores[i].length; x < xx; x++){
    if(uniqueDaysScores[i][x] > 1000){
      scoreSortedPrimer.unshift(uniqueDaysScores[i][x]);
    }
  }
  uniqueDaysScores.push(scoreVarianceSortedPrimer);
}

The uniqueDaysScores array will contain all the arrays with the date first and the lowest and highest score thereafter. I have added comments to make it easier to follow. It kind of seems like a long process and I'm just curious if there is a more experienced programmer that can see where this script can be optimized or improved.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You state that the date is always in the first position, but your code thinks it can be at any position. Which is it? If it's always first then you've got a nice simplification off the bat. Also, can the same date occur twice in the input Array? And does the output Array need to be sorted by date? \$\endgroup\$ – twhb Jan 8 '16 at 10:16
6
\$\begingroup\$

I'm a little unsure of why scoresDates starts with a 56-item array and the rest are 2-item arrays.

Just to be sure, this is your data model, correct? Please let me know if I've totally misread things.

scoresDates is [
    [date, score, score, score...],
    [date, score],
    [date, score],
    ...
]

Working forwards from your current code, here are some suggestions. I've used some functions like Array.filter that require IE > 9; if you need to support old Internet Explorer then a utility library like Lodash would provide polyfills for these.

In general, you're doing more loops through the data than should be necessary. For example:

Sorting is extra work

  // Next step is sorting the scoreSorted1 array from lowest score to highest score.
  scoreSorted1.sort(function(a,b){
    return a-b;
  });

  // Find maximum and minimum scores in the array and finally push them into the scoreSortedPrimer array.
  var primerMin = Math.min.apply(Math, scoreSorted1);
  scoreSortedPrimer.push(primerMin);
  var primerMax = Math.max.apply(Math,scoreSorted1);
  scoreSortedPrimer.push(primerMax);

scoreSorted1 seems to only be used for calculating min and max. Sorting is more work than min and max, each of which go through the entire array anyway. Here are a few faster options:

  • keep the sorting, in which case you will know that min is the first item and max is the last
  • don't sort, just call Math.min and Math.max.
  • write your own function that calculates both min and max in just one pass of the array, as opposed to two (Math.min + Math.max)

Array.filter

Using higher-level functions like forEach and filter can simplify some of your loops, by letting you ignore array indexes.

For example, this:

var scoreSorted1 = [];

// First level for loop that goes through the unique scores array and ignores the date, while pushing all the scores into the scoreSorted1 array.

for(var z = 0, zz = uniqueDaysScores[i].length; z < zz; z++){
  if(uniqueDaysScores[i][z] < 1000){
    scoreSorted1.push(uniqueDaysScores[i][z]);
  }
}

becomes

var scoreSorted1 = uniqueDaysScores[i].filter(function(score) {
  return score < 1000;
});

And, at a higher level, your entire program becomes two parts: merging all the scores for each date, then applying a function [scores] -> [min(scores), max(scores)] to each item in that merged object.

Comments

Comments should explain why, rather than describe the code, unless it is particularly hairy. This is a little subjective, but, for example, you can easily read scoreSorted1.sort(... and see that it is sorting the scores, comment or not.

Instead, I recommend saving comments for more confusing passages and for higher-level overviews of your functions or logic. Using descriptive variable and function names is often enough of a comment.

// Logic to take all the scores, sort them from decreasing to increasing order, then package the date, lowest and shortest score into the finished array, scoreSorted.

for(var i = 0, ii=uniqueDaysScores.length; i < ii; i++){
  var scoreSorted1 = [];
  var scoreSortedPrimer = [];

// First level for loop that goes through the unique scores array and ignores the date, while pushing all the scores into the scoreSorted1 array.

  for(var z = 0, zz = uniqueDaysScores[i].length; z < zz; z++){
    if(uniqueDaysScores[i][z] < 1000){
      scoreSorted1.push(uniqueDaysScores[i][z]);
    }
  }

  // Next step is sorting the scoreSorted1 array from lowest score to highest score.
  scoreSorted1.sort(function(a,b){
    return a-b;
  });

  // Find maximum and minimum scores in the array and finally push them into the scoreSortedPrimer array.
  var primerMin = Math.min.apply(Math, scoreSorted1);
  scoreSortedPrimer.push(primerMin);
  var primerMax = Math.max.apply(Math,scoreSorted1);
  scoreSortedPrimer.push(primerMax);

  // Finally, find the date and push it into the scoreSortedPrimer array, so it can be pushed into the main scoreSorted array.

  for(var x = 0, xx = uniqueDaysScores[i].length; x < xx; x++){
    if(uniqueDaysScores[i][x] > 1000){
      scoreSortedPrimer.unshift(uniqueDaysScores[i][x]);
    }
  }
  uniqueDaysScores.push(scoreVarianceSortedPrimer);
}

becomes, more sparsely,

// Calculate min and max score for each date, appending to uniqueDaysScores.

uniqueDaysScores.forEach(function(dateAndScores) {

  var date = dateAndScores.filter(function(score) {
    return score >= 1000;
  })[0];

  var scores = dateAndScores.filter(function(score) {
    return score < 1000;
  });

  uniqueDaysScores.push([
      date,
      Math.min.apply(Math, scores),
      Math.max.apply(Math, scores)
  ]);
});

and would be even simpler if the date was stored somewhere specific so that you don't need to use loops or filter to find it.

Let's rewrite the initial score-merging step to accommodate this.

Merging/grouping scores for each date

I think this is where a lot of the extra complexity is coming from.

Using an object, with the dates as keys and the arrays of scores as values, would be natural, but your array of [date, scores...] arrays would also work.

The important thing is to make sure the date is easy to find, not mixed into your scores, so you don't need to search for it later (by checking what score is > 1000).

// Logic that stacks all the scores per unique day from all the user's game scores. All unique dates with subsequent scores are stored in the uniqueDaysScores array.

uniqueDaysScores = [];
uniqueDaysScores.push(scoresDates[0]);

for(var i = 0, ii = scoresDates.length; i < ii; i++){
  var tempArray = [];

  if(scoresDates[i][0] == uniqueDaysScores[0][0]){
    uniqueDaysScores[0].push(scoresDates[i][1]);
  } else{
    tempArray.push(scoresDates[i][0]);
    tempArray.push(scoresDates[i][1]);
    uniqueDaysScores.unshift(tempArray);
  }
}

becomes, using an object of {date: scores},

// Group scores by date

var scoresByDate = {};

scoresDates.forEach(function(scores) {
  var date = scores[0];
  scores.shift();  // Remove date

  if (!(date in scoresByDate)) {
    scoresByDate[date] = scores;
  } else {
    scoresByDate[date].concat(scores);
  }
}

I've used forEach here, but your original loop-with-index would also work just fine.

With this scoresByDate structure, finding min and max for each date becomes very easy:

var maxMinByDate = Object.keys(scoresByDate).map(function(date) {
  return scoreMinMax(date, scoresByDate[date]);
};

or you could just not bother with a separate scoreMinMax function.


Altogether

With fewer, more high-level comments and a better data model, you end up with something more like this:

var scoresDates = [...];

// Group scores by date.

var scoresByDate = {};

scoresDates.forEach(function(scores) {
  var date = scores[0];
  scores.shift();  // Remove date

  if (!(date in scoresByDate)) {
    scoresByDate[date] = scores;
  } else {
    scoresByDate[date].concat(scores);
  }
}

// Calculate min and max score for each date.

var minMaxByDate = Object.keys(scoresByDate).map(function(date) {
  var scores = scoresByDate[date];
  return [
    date,
    Math.min.apply(Math, scores),
    Math.max.apply(Math, scores)
  ];
});

which could be further accelerated by writing a extrema(arr) function that would return the pair [min(arr), max(arr)] in one pass.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.