patternjavascriptMinor
Summarize JS Array more efficiently
Viewed 0 times
summarizeefficientlymorearray
Problem
I am working on a project for Udacity, where I should use CSV files to display data on fake corporate dashboard. Structure of the data I am working on is like this:
I am using AngularJS for this project, I created service for getting the data, formatting it to JSON and summarizing data into months. It works good, but I can see that it takes up to 1-2 sec to load a page, and data sheet has only around 90 lines. Anybody has suggestion how to speed it up?
I am also using MomentJS for formatting dates.
Service:
Controller:
```
issuesdataProvider.summorizeIssues().then(function(data)
I am using AngularJS for this project, I created service for getting the data, formatting it to JSON and summarizing data into months. It works good, but I can see that it takes up to 1-2 sec to load a page, and data sheet has only around 90 lines. Anybody has suggestion how to speed it up?
I am also using MomentJS for formatting dates.
Service:
var getIssues = function() {
return $http.get('data/issues.csv').then(
function(response) {
return CSV2JSON(response.data);
}
);
};
this.summorizeIssues = function() {
return new Promise(function(resolve) {
getIssues().then(function(data) {
var datesarray = [];
var returnarray = [];
data.forEach(function(item) {
if (item['submission timestamp']) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (datesarray.indexOf(month) < 0) {
datesarray.push(month);
}
}
});
datesarray.sort(function(a, b) {
return moment(a, 'MMM-YY') - moment(b, 'MMM-YY');
});
datesarray.forEach(function(date) {
var counter = 0;
data.forEach(function(item) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (month === date) {
counter += 1;
}
});
returnarray.push({
label: date,
issues: counter
});
});
resolve(returnarray);
});
});
};Controller:
```
issuesdataProvider.summorizeIssues().then(function(data)
Solution
I am also using MomentJS for formatting dates.
I have experienced performance issues when using MomentJS before. It's not because it's terrible but rather the opposite. It just does a lot of things to get dates right. But you'll have to look out for potential bottlenecks.
it takes up to 1-2 sec to load a page
Also consider that you're using AJAX. Network latency can become an issue as well. But there's nothing you can do with that since you have no control of the network. Consider other strategies like loading pages, or caching, or data compression.
We can do nothing with a slow network, but we can do something about the slow code.
The lag you are experiencing is most likely the browser's garbage collector kicking in. Every
The goal here is to reduce the usage of
The code above can be further tuned by using regular loops instead of array methods, grouping up loop operations, use pushing to existing arrays instead of returning new arrays after each oepration. In summary, avoid calling out
I have experienced performance issues when using MomentJS before. It's not because it's terrible but rather the opposite. It just does a lot of things to get dates right. But you'll have to look out for potential bottlenecks.
it takes up to 1-2 sec to load a page
Also consider that you're using AJAX. Network latency can become an issue as well. But there's nothing you can do with that since you have no control of the network. Consider other strategies like loading pages, or caching, or data compression.
We can do nothing with a slow network, but we can do something about the slow code.
data.forEach(function(item) {
if (item['submission timestamp']) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (datesarray.indexOf(month) < 0) {
datesarray.push(month);
}
}
});
datesarray.sort(function(a, b) {
return moment(a, 'MMM-YY') - moment(b, 'MMM-YY');
});
datesarray.forEach(function(date) {
var counter = 0;
data.forEach(function(item) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (month === date) {
counter += 1;
}
});
returnarray.push({
label: date,
issues: counter
});
});The lag you are experiencing is most likely the browser's garbage collector kicking in. Every
moment call creates a new MomentJS object. It's terrible thing to do in loops. In your case, you call it on each of the loops you have. You have 1 loop that executes n times, 1 sort whose callback is called an arbitrary amount of times, and 2-level loop which is n^2.The goal here is to reduce the usage of
moment and avoiding the use of nested loops. The following code is my attempt at it. It uses moment only once to grab everything it needs into a regular object. Everything else is just linear object/array operations.const issuesByDate = data.reduce((data, item) => {
// Return early when there's no timestamp
if(!item['submission timestamp']) return data;
// Group the data by using an object
// { 'MMM-YY': { label: ..., timestamp: ..., issues: ... }}
// The only time we use moment
const itemMoment = moment(item['submission timestamp'])
const label = itemMoment.format('MMM-YY');
const timestamp = itemMoment.valueOf();
// Use the label as key to group similar items
// Append timestamp, a number value. Well need it later for sorting
data[label] = data[label] || { label, timestamp, issues: 0 };
data[label].issues += 1;
return data;
}, {});
// Convert object into an array
// [{ label: ..., timestamp: ..., issues: ... }]
return Object.keys(issuesByDate)
.map(key => issuesByDate[key])
// Sort here since object keys don't guarantee order
.sort((a, b) => a.timestamp - b.timestamp)
// Strip off the timestamp property that we don't need
// [{ label: ..., issues: ... }]
.map(item => ({ label: item.label, issues: item.issues }))The code above can be further tuned by using regular loops instead of array methods, grouping up loop operations, use pushing to existing arrays instead of returning new arrays after each oepration. In summary, avoid calling out
moment too often and avoid nested loops.Code Snippets
data.forEach(function(item) {
if (item['submission timestamp']) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (datesarray.indexOf(month) < 0) {
datesarray.push(month);
}
}
});
datesarray.sort(function(a, b) {
return moment(a, 'MMM-YY') - moment(b, 'MMM-YY');
});
datesarray.forEach(function(date) {
var counter = 0;
data.forEach(function(item) {
var month = moment(item['submission timestamp'], 'M-D-YY h:m a').format('MMM-YY');
if (month === date) {
counter += 1;
}
});
returnarray.push({
label: date,
issues: counter
});
});const issuesByDate = data.reduce((data, item) => {
// Return early when there's no timestamp
if(!item['submission timestamp']) return data;
// Group the data by using an object
// { 'MMM-YY': { label: ..., timestamp: ..., issues: ... }}
// The only time we use moment
const itemMoment = moment(item['submission timestamp'])
const label = itemMoment.format('MMM-YY');
const timestamp = itemMoment.valueOf();
// Use the label as key to group similar items
// Append timestamp, a number value. Well need it later for sorting
data[label] = data[label] || { label, timestamp, issues: 0 };
data[label].issues += 1;
return data;
}, {});
// Convert object into an array
// [{ label: ..., timestamp: ..., issues: ... }]
return Object.keys(issuesByDate)
.map(key => issuesByDate[key])
// Sort here since object keys don't guarantee order
.sort((a, b) => a.timestamp - b.timestamp)
// Strip off the timestamp property that we don't need
// [{ label: ..., issues: ... }]
.map(item => ({ label: item.label, issues: item.issues }))Context
StackExchange Code Review Q#133766, answer score: 5
Revisions (0)
No revisions yet.