Convert task data into a flat structure

Posted on

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]; and dateConvert.unshift("x"); which could have been dateConvert = ["x", ...dateConvert];, the only good reason for unshift could have been that dateConvert is a const but it isnt
  • I dont like x as a variable name, it says nothing to me, I would go for o (object)
  • Similarly when you reduce allTasks you pass arr as the cumulator, but it’s not an array, it’s an object so you could/should use o 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 like group all tasks properties
  • I guess a task is an item, but I would rather see allTasks.reduce((o, task) than allTasks.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)

Leave a Reply

Your email address will not be published. Required fields are marked *