2
\$\begingroup\$

I know my solution works, but I would like to know if the function can be simplified so that the loop is not called twice. Is there a way to pass the properties to be run through as parameters in the function?

My given data structure looks like (in which totalQuantities and typeQuantities might contain an uncertain amount of objects):

data = [
  {
    "totalQuantities": [
      {
        "ammount": 23
      }
    ],
    "typeQuantities": [
      {
        "type": {
          "id": 0,
          "name": "Test"
        },
        "singleQuantities": [
          {
            "ammount": 45
          }
        ]
      }
    ]
  }
]

My desired output is:

values = [
    {type: 'Total', ammount1: 23},
    {type: 'Test1', ammount1: 45}
]

And my solution - of which I'm wondering if there is another way to do so - is:

transformData(data) {
  const values = [];
  // loop through totalQuantities
  const objTotal: any = {};
  objTotal.type = 'Total';
  for (let i = 0; i < data.totalQuantities.length; i++) {
    const name = 'ammount' + (i + 1).toString();
    objTotal[name] = data.totalQuantities[i].ammount;
  }
  values.push(objTotal);
  // loop through each element of singleQuantities
  data.typeQuantities.map(element => {
    const obj: any = {};
    obj.type = element.type.name;
    for (let i = 0; i < element.singleQuantities.length; i++) {
      const name = 'ammount' + (i + 1).toString();
      obj[name] = element.singleQuantities[i].ammount;
    }
    values.push(obj);
  });
  return values;
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You have a typo: ammount instead of amount. \$\endgroup\$ Nov 30, 2020 at 15:48

1 Answer 1

5
\$\begingroup\$

It looks like you're using TypeScript. TypeScript is great - it can turn difficult-to-debug runtime errors into trivially fixable compile errors. But for this sort of type checking to work:

Don't use any, since it effectively disables type checking for that expression. TSLint rule: no-any. Better to think about what the data structure will be, and type the expression appropriately.

Proper spelling is more professional and helps prevent bugs. Consider changing all the occurrences of ammount to amount.

Use an array instead of numerically indexed properties in an object, if at all possible - it'll make more sense as a data structure and will make iteration over it easier. For example, instead of having something like:

{type: 'Total', ammount1: 23, ammount2: 99, ammount3: 555},

would it be possible to tweak the user of the object to accept instead:

{type: 'Total', amounts: [23, 99, 555]}

? If I were you, I'd exert a significant amount of effort into making this change possible; it'd be a good structural improvement.

Use .map only when constructing an array by transforming every element of another array - if what you do inside the map is side-effects like .push, it would be more appropriate to use a generic iteration method like .forEach. But using .map and returning an object will make the code easier:

const data=[{totalQuantities:[{amount:23}],typeQuantities:[{type:{id:0,name:"Test"},singleQuantities:[{amount:45}]}]}];

const transformData = (data) => {
  const objTotal = {
    type: 'total',
    amounts: data.totalQuantities.map(({ amount }) => amount)
  };
  const typeQuantitiesObjs = data.typeQuantities.map(element => ({
    type: element.type.name,
    amounts: element.singleQuantities.map(({ amount }) => amount)
  }));
  return [objTotal, ...typeQuantitiesObjs];
};

console.log(transformData(data[0]));

TypeScript works really well when all of your data transformation is pure, as done in the above snippet. When you construct arrays and objects outright instead of mutating them inside a loop, TypeScript can often infer the resulting types automatically, which means that the code doesn't have much type-related syntax, and can look just like JavaScript (and so is easy to read) - despite still being type-safe.

so that the loop is not called twice

I think the two data sources are too different for this to be done in an easy-to-understand way: one uses type: 'total', the other uses type: element.type.name,. Also, one uses .totalQuantities to get the quantities, and the other uses .singleQuantities.

While it would be possible to refactor to avoid the repetition of { type: .., amounts: ... }, it would look somewhat convoluted; I wouldn't recommend it, it'll be harder to make sense of at a glance.

If the output objects must contain amount1 etc instead of a single array, I'd make a helper function that can transform an array of input quantities objects into a properly formatted object with Object.fromEntries, then call that function for both the total and the typeQuantities:

const data=[{totalQuantities:[{amount:23}],typeQuantities:[{type:{id:0,name:"Test"},singleQuantities:[{amount:45}]}]}];

const getAmounts = quantities => Object.fromEntries(
  quantities.map(
    ({ amount }, i) => ['amount' + (i + 1), amount]
  )
);
const transformData = (data) => {
  const objTotal = {
    type: 'total',
    ...getAmounts(data.totalQuantities)
  };
  const typeQuantitiesObjs = data.typeQuantities.map(element => ({
    type: element.type.name,
    ...getAmounts(element.singleQuantities)
  }));
  return [objTotal, ...typeQuantitiesObjs];
};

console.log(transformData(data[0]));

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for your input. I surely agree with you on the typing and spelling. Unluckily the output has to be an object like '{type: 'Total', amount1: 23, amount2: 99, amount3: 555}' because it is passed on to a table which requires this kind of structure. Anyway it helps a lot to see that even more experienced coders recommend to keep a certain value of readability to the code! \$\endgroup\$ Dec 1, 2020 at 7:07
  • \$\begingroup\$ If the data is used for display immediately instead of being passed along to some other data processor, having separate amount1 etc properties sounds fine to me. \$\endgroup\$ Dec 1, 2020 at 15:21
  • \$\begingroup\$ Thank you! You're solution looks much more structured than mine - that was, what I was looking for \$\endgroup\$ Dec 2, 2020 at 7:19

Your Answer

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

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