HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Convert data (adding and sorting) with JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sortingconvertwithjavascriptaddinganddata

Problem

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 thi

Solution

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
/ 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%



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:

Code Snippets

{
  2010: {
    a: 32,
    b: 40,
    c: 30
  },
  2011: {
    a: 43,
    c: 64
  },
  2012: {
    a: 40,
    b: 65,
    c: 45
  },
  2013: {
    b: 45,
    c: 30
  }
}
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
}
for(var i = 0; i < data.length; i++) {
  if(namesFound[data[i][0]]) { continue; }
  nameArr.push(data[i][0]);
  namesFound[data[i][0]] = true;
}
for (var i = 0; i < data.length; i++) {
  if (!namesFound[data[i][0]]) {
    nameArr.push(data[i][0]);
    namesFound[data[i][0]] = true;
  }
}
for (var i = 0; i < data.length; i++) {
for (var i = 0; i < nameArr.length; i++) {

Context

StackExchange Code Review Q#160843, answer score: 4

Revisions (0)

No revisions yet.