5
\$\begingroup\$

I have an array of data like this:

var data = [
    ['a', 2010, 12], ['a', 2010, 20], ['a', 2011, 43], ['a', 2012, 25], ['a', 2012, 15],
    ['b', 2010, 40], ['b', 2012, 65], ['b', 2013, 20], ['b', 2013, 10], ['b', 2013, 15],
    ['c', 2010, 13], ['c', 2010, 17], ['c', 2011, 22], ['c', 2011, 32], ['c', 2011, 10], ['c', 2012, 45], ['c', 2013, 10], ['c', 2013, 20]
]

and each array data is [name, year, value].

What I want to do here is to add values when name and year are the same and then sort data by year and change the order by name, so I want to convert data to:

var newData = [
    ['a', 2010, 32], ['b', 2010, 40], ['c', 2010, 30],
    ['a', 2011, 43], ['c', 2011, 64]
    ['a', 2012, 40], ['b', 2012, 65], ['c', 2012, 45],
    ['b', 2013, 45], ['c', 2013, 30]
]

I am doing this by taking multiple steps:

// find names
var sortByName = [];
var nameArr = [];
var namesFound = {};
for(var i = 0; i < data.length; i++) {
  if(namesFound[data[i][0]]) { continue; }
  nameArr.push(data[i][0]);
  namesFound[data[i][0]] = true;
}
// sort data by name by creating an array for each name (2D array to 3D array)
for (var i = 0; i < nameArr.length; i++) {
  var result = data.filter(function(arr) {
    return arr[0] == nameArr[i]
  })
  sortByName.push(result)
}
// add values with a same year
var newArr = sortByName.map(function(eachArr) {
  var sumBySameYear =  eachArr.reduce(function(acc, arr) {
    if (acc.year === arr[1]) {
      acc.result[acc.result.length - 1][2] += arr[2];
    } else {
      acc.result.push(arr);
      acc.year = arr[1]
    }
    return acc
  }, {result: [], year: null});
  return sumBySameYear.result
});
// 3D array to 2D array
var multiToSingleArr = [].concat.apply([], newArr);
// then sort new data by year and keep the order of name
var newData = multiToSingleArr.slice().sort(function(a, b) {
  return a[1] - b[1] || a[0].localeCompare(b[0]);
});

I definitely feel like I am taking unnecessary steps here, so how can I make this more concise?

\$\endgroup\$
1
  • \$\begingroup\$ Do you need to keep that same data structure? Particularly, the inner arrays seem more suited to be object literals. Making this simple change would allow you to greatly simply this code. \$\endgroup\$
    – Mike Brant
    Apr 15, 2017 at 15:43

2 Answers 2

4
\$\begingroup\$

My solution

I came with something more concise, readable and effective:

const convertData = data => {
  const obj    = {}, // Intermediate object {year => {letter => number}}
        result = [];

  data.forEach(array => {
    const [letter, year, number] = array;
    obj[year]         = obj[year] || {}; // Creating object if it doesn't exist
    obj[year][letter] = obj[year][letter] + number || number; // Create or increment property
  });

  Object.keys(obj).forEach(year => {
    Object.keys(obj[year]).forEach(letter => {
      result.push([letter, parseInt(year), obj[year][letter]]);
    });
  });
  
  return result;
};

/* ===== DEMO ===== */

const data = [
    ['a', 2010, 12], ['a', 2010, 20], ['a', 2011, 43], ['a', 2012, 25], ['a', 2012, 15],
    ['b', 2010, 40], ['b', 2012, 65], ['b', 2013, 20], ['b', 2013, 10], ['b', 2013, 15],
    ['c', 2010, 13], ['c', 2010, 17], ['c', 2011, 22], ['c', 2011, 32], ['c', 2011, 10], ['c', 2012, 45], ['c', 2013, 10], ['c', 2013, 20]
];

console.log(JSON.stringify(convertData(data)));
/* Output console formatting */
.as-console-wrapper { top: 0; }

Explanation

First part of my function (data.forEach()) builds intermediate object obj that looks like this afterwards:

{
  2010: {
    a: 32,
    b: 40,
    c: 30
  },
  2011: {
    a: 43,
    c: 64
  },
  2012: {
    a: 40,
    b: 65,
    c: 45
  },
  2013: {
    b: 45,
    c: 30
  }
}

The second part just iterates by years and for every year by it's letters and builds result array.

Optimized version

First part of my function (data.forEach() loop) can be replaced with

for (let i = 0; i < data.length; i++) {
  const [letter, year, number] = data[i];
  obj[year]         = obj[year] || {}; // Creating object if it doesn't exist
  obj[year][letter] = obj[year][letter] + number || number; // Create or increment property
}

It's simply change to the for loop, but it gives significant performance boost without compromising readability at all. Interestingly, despite .forEach()es are generally slower, replacing the rest of the code with regular loops has negative impact on performance.

ES5 version

function convertData(data) {
  var obj    = {}, // Intermediate object {year => {letter => number}}
      result = [];

  for (var i = 0; i < data.length; i++) {
    var letter = data[i][0],
        year   = data[i][1],
        number = data[i][2];
    obj[year]         = obj[year] || {}; // Creating object if it doesn't exist
    obj[year][letter] = obj[year][letter] + number || number; // Create or increment property
  };

  Object.keys(obj).forEach(function objForEach(year) {
    Object.keys(obj[year]).forEach(function yearForEach(letter) {
      result.push([letter, parseInt(year), obj[year][letter]]);
    });
  });
  
  return result;
}

/* ===== DEMO ===== */

var data = [
    ['a', 2010, 12], ['a', 2010, 20], ['a', 2011, 43], ['a', 2012, 25], ['a', 2012, 15],
    ['b', 2010, 40], ['b', 2012, 65], ['b', 2013, 20], ['b', 2013, 10], ['b', 2013, 15],
    ['c', 2010, 13], ['c', 2010, 17], ['c', 2011, 22], ['c', 2011, 32], ['c', 2011, 10], ['c', 2012, 45], ['c', 2013, 10], ['c', 2013, 20]
];

console.log(JSON.stringify(convertData(data)));
/* Output console formatting */
.as-console-wrapper { top: 0; }

Benchmark

  • @Flambino's solution:      17,241 ops/s ±1.2%
  • My solution:                      19,433 ops/s ±3.02%
  • My solution (optimized):  33,490 ops/s ±3.25% Benchmark

Remarks to your code

Formatting

There are a few minor formatting problems:

  • You could combine multiple vars together,
  • Lack of spaces here and there, and multiple ones in other places,
  • Few semicolons are missing (despite optional, it's a good practice to have them),
  • Minor indentation issues.

Also you could add more spacing to all your blocks of code to aid readability.

Unnecessary continue

continue is very often a useful statement, although it shouldn't be used if it can be avoided without compromising readability. This in my opinion

for(var i = 0; i < data.length; i++) {
  if(namesFound[data[i][0]]) { continue; }
  nameArr.push(data[i][0]);
  namesFound[data[i][0]] = true;
}

could be replaced with

for (var i = 0; i < data.length; i++) {
  if (!namesFound[data[i][0]]) {
    nameArr.push(data[i][0]);
    namesFound[data[i][0]] = true;
  }
}

Loop iterator declaration

These two lines suggest that you treat variable i like if it had block scope, which is not the case:

for (var i = 0; i < data.length; i++) {
for (var i = 0; i < nameArr.length; i++) {

The way the code looks right now, after i is declared for the first time, it's available everywhere in the function containing it, or worse yet ― if it's all in a global scope, it's global variable. Block scope variable can be declared with ES6's let. If you don't want to or can't use it, you could declare i before both loops, and then go with i = 0 without var keyword.

Use strict equality operator whenever possible

This line

return arr[0] == nameArr[i];

could be easily replaced with

return arr[0] === nameArr[i];

That way no type conversion will be performed.

Don't declare functions within loops

You declare function withing your loops in your code. That way on each iteration you in fact create a completely new function.

Name your anonymous functions

Named function expressions may come in handy when it comes to debugging. Quoting "Named function expressions demystified" by Juriy "kangax" Zaytsev:

In a nutshell, named function expressions are useful for one thing only — descriptive function names in debuggers and profilers.

Don't push it

sortByName[i] = result;

would be much more efficient than

sortByName.push(result);

Here's the benchmark:

Array append benchmark

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Aside from providing an alternative solution, do you find that there is anything to actually review about the OP's code? Also, the OP is using ECMAScript 5, while this answer uses ECMAScript 6, perhaps this would warrant some clarification, as the OP's environment might not support ES6. \$\endgroup\$
    – Phrancis
    Apr 15, 2017 at 22:13
  • \$\begingroup\$ @Phrancis'DROPTABLEUsers-- The only ES6 elements in here are consts, arrow syntax and destructuring assignments, so mostly syntax sugar, although it's true that I should provide fallback ES5 code, or at least mention Babel (or other) transpiler. I of course have few comments about OP's code too, but it's pretty late where I am right now, so I will add them tomorrow. By the way, nice user name ;). \$\endgroup\$
    – Przemek
    Apr 15, 2017 at 22:33
  • 1
    \$\begingroup\$ wow! thank you so much for your solution and advise! really appreciate it. \$\endgroup\$
    – kay
    Apr 16, 2017 at 10:50
0
\$\begingroup\$

I would think you can get by with this:

// step 1: Combine same year & letter in a object
const combined = data.reduce((memo, array) => {
  const [letter, year, value] = array;
  const key = `${letter}/${year}`; // create a suitable key
  if(memo[key]) {
    memo[key][2] += value; // add value if key already exists
  } else {
    memo[key] = array; // otherwise, set the key
  }
  return memo;
}, {});

// step 2: get values and sort them
const sorted = Object.keys(combined)
  .map(key => combined[key])
  .sort((a, b) => a[1] - b[1] || a[0].localeCompare(b[0]));

It skips the "get all names" and extra sorting step. Instead it creates keys from each array in the data ("a/2010", "b/2013", etc.), and uses that to sum values for identical keys. This is stored in a object, whose values are then sorted - same way as your original code did - in step 2.

\$\endgroup\$
2
  • \$\begingroup\$ Aside from providing an alternative solution, do you find that there is anything to actually review about the OP's code? Also, the OP is using ECMAScript 5, while this answer uses ECMAScript 6, perhaps this would warrant some clarification, as the OP's environment might not support ES6. \$\endgroup\$
    – Phrancis
    Apr 15, 2017 at 22:13
  • \$\begingroup\$ @Phrancis'DROPTABLEUsers-- Got called away, so for once I took the easy route, and stopped at alternative code. I'll get back to it asap \$\endgroup\$
    – Flambino
    Apr 15, 2017 at 22:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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