Problem
I have an array of data in the below format
let input = [
{
time: "2020-7",
tasks: [
{
key: "p1",
value: 15
},
{
key: "p2",
value: 13
},
]
},
{
time: "2020-8",
tasks: [
{
key: "p1",
value: 16
},
{
key: "p2",
value: 19
},
]
},
{
time: "2020-9",
tasks: [
{
key: "p1",
value: 12
},
{
key: "p2",
value: 93
},
]
}
]
After config input array, then ‘dataConvert’ is formatted as follows:
dataConvert = [
["x","2020-7", "2020-8", "2020-9"],
["p1", 15, 16, 12],
["p2", 13, 19, 93]
]
I tried use reduce
, I want to find another way of optimizing.
let dateConvert = [], valueConvert = [];
data2.forEach(x=>{
let date = new Date(x.time);
if (date) {
let getYear = date.getFullYear();
let getMonth = date.getMonth() + 1;
let newDate = `${getYear}-${getMonth}-1`;
return dateConvert = [...dateConvert, newDate];
}
})
dateConvert.unshift("x");
// get p1 p2 value
let allTasks = data2.flatMap(x => x.tasks);
valueConvert = Object.values(allTasks.reduce((arr, item) => {
arr[item.key] = arr[item.key] || [item.key];
arr[item.key].push(item.value);
return arr;
}, {}));
dataConvert = [...[dateConvert], ...valueConvert];
thank u.
Solution
From a short review;
- The code does not produce exactly your output, the
x
part is different;
[“x”, “2020-7-1”, “2020-8-1”, “2020-9-1”], I assume the code is right - That
reduce
is fine, it’s the best part of the code really - The date part seems complicated the check for the valid date could have been done in a filter, the rest in a map
- Consistency is important, you use both the fancy technique
dataConvert = [...[dateConvert], ...valueConvert];
anddateConvert.unshift("x");
which could have beendateConvert = ["x", ...dateConvert];
, the only good reason forunshift
could have been thatdateConvert
is aconst
but it isnt - I dont like
x
as a variable name, it says nothing to me, I would go foro
(object) - Similarly when you reduce
allTasks
you passarr
as the cumulator, but it’s not an array, it’s an object so you could/should useo
there - This
[...[dateConvert], ...valueConvert];
is equivalent to[dateConvert, ...valueConvert];
, you may want to play around more with the spread operator get p1 p2 value
is a very contemporary comment, it is true today because tasks indeed only contain p1 and p2, but the day that tasks get new or changed, that comment will be worthless, consider something likegroup all tasks properties
- I guess a task is an item, but I would rather see
allTasks.reduce((o, task)
thanallTasks.reduce((arr, item)
If you take all that, I would counter propose this;
function convertData(data){
let dateConvert = data.filter(o => new Date(o.time)).map(o => {
const date = new Date(o.time);
return `${date.getFullYear()}-${ date.getMonth() + 1}-1`;
});
dateConvert = ["x", ...dateConvert];
let allTasks = data.flatMap(x => x.tasks);
//Group all task properties
const valueConvert = Object.values(allTasks.reduce((o, task) => {
o[task.key] = o[task.key] || [task.key];
o[task.key].push(task.value);
return o;
}, {}));
return [dateConvert, ...valueConvert];
}
const data2 = [
{
time: "2020-7",
tasks: [
{
key: "p1",
value: 15
},
{
key: "p2",
value: 13
},
]
},
{
time: "2020-8",
tasks: [
{
key: "p1",
value: 16
},
{
key: "p2",
value: 19
},
]
},
{
time: "2020-9",
tasks: [
{
key: "p1",
value: 12
},
{
key: "p2",
value: 93
},
]
}
];
console.log(convertData(data2));
You have
valueConvert = Object.values(allTasks.reduce((arr, item) => {
arr[item.key] = arr[item.key] || [item.key];
arr[item.key].push(item.value);
return arr;
}, {}));
This is somewhat opinion-based, but I’d recommend avoiding reduce in these sorts of situations, where the accumulator is the same object throughout all iterations. See that link above for a video by some of Chrome’s developers going into more detail on why it’s not the best idea. In short, while reduce
is a possible method to solve the problem, it’s unnecessarily verbose and can be a bit confusing (especially for readers of your code who may not be sufficiently familiar with reduce
) compared to a plain loop; it’s needlessly complex. Don’t feel like you have to use array methods everywhere you can (even if using it shows that you know how to use .reduce
).
You could change it to:
const tasksByKey = {};
for (const { key, value } of arr) {
tasksByKey[key] = tasksByKey[key] || [key];
tasksByKey[key].push(value);
}
const valueConvert = Object.values(tasksByKey);
You use let
a lot. Best to always use const
when possible – this makes code easier to read and reason about, when you know at a glance that a particular variable name will never be reassigned. Consider using a linter, if you aren’t already doing so – it can make code style more consistent and readable, and can reduce bugs.
Similarly, if a variable name doesn’t have to be reassigned, consider avoiding doing so. Rather than
dataConvert = [...[dateConvert], ...valueConvert];
you might use
const datesAndValues = [dateConvert, ...valueConvert];
Remember that .flatMap
is a pretty new method – if this code is running on a public-facing website, make sure to include a polyfill. (Object.values
was only officialized in ES2017, so you’ll probably want a polyfill for it too)