Sum up nested array of objects based on keys

Question

I have this function that should take some array of objects and sum up the values of the inner objects based on some keys, and conditionally add to it some values from another object.

Is there a way to optimize this code?

 function sumUpKeys(objects, totals = undefined) {
 const totalsSum = totals?.reduce((totalsAcc, currentTotalsItem) => {
 return {
 key1:
 (totalsAcc.key1 || 0) +
 (this.computeVal(
 currentTotalsItem.num || 0,
 currentTotalsItem.gross
 ) || 0),
 key2:
 (totalsAcc.key2 || 0) +
 (this.computeVal(
 currentTotalsItem.num || 0,
 currentTotalsItem.net
 ) || 0),
 };
 }, {});
 const objectsSum = objects.reduce((objectsAcc, currentObject) => {
 const itemsSum = currentObject.items.reduce((itemsAcc, currentItem) => {
 return {
 key1:
 (this.computeVal(currentObject.num, currentItem.key1) || 0) +
 (itemsAcc.key1 || 0),
 key2:
 (this.computeVal(currentObject.num, currentItem.key2) || 0) + (
 itemsAcc.key2 || 0),
 };
 }, {});
 objectsAcc = {
 key1: (objectsAcc.key1 || 0) + (itemsSum.key1 || 0),
 key2: (objectsAcc.key2 || 0) + (itemsSum.key2 || 0),
 };
 return objectsAcc;
 }, {});
 // add additional services sum to the monthly charges sum
 return {
 key1: (objectsSum.key1 || 0) + (totalsSum?.key1 || 0),
 key2: (objectsSum.key2 || 0) + (totalsSum?.key2 || 0),
 };
 }
function computeVal(num, val) {
 return val ? val * num : undefined;
 }
 
 
 const objects1 = [
 {
 num: 3,
 items: [
 {
 key1: 4,
 key2: 2,
 },
 {
 key1: 10,
 key2: 20,
 },
 ],
 key3: 'aa',
 },
 {
 num: 4,
 items: [
 {
 key1: 4,
 key2: 2,
 },
 ],
 key3: 'aa',
 },
];
const totals1 = [
 {
 num: 2,
 gross: 10,
 net: 5,
 },
 {
 num: 3,
 gross: 6,
 net: 12,
 },
];
// result with totals
const result1 = sumUpKeys(objects1, totals1);
// result without totals
const result2 = sumUpKeys(objects1);
console.log(result1);
console.log(result2);

• Dave Meehan: Can you explain what you want to achieve by 'optimising'. Do you mean to run faster, use less memory, be easier to 'read', some combination, something else?
• callback: @DaveMeehan yes, maybe somehow avoid the nested reduce if it's possible at all? Or have fewer lines of code.

Answer 1

You are returning a new object in each iteration inside the reducers, when you could use just one object for the tallies (see below). The handling of undefined values suggest to me that the underlying data might be invalid, in which case it makes more sense to fix the data and establish validation before insert. Unless of course the values are spec'd to be optional.

const sumKeys = (xs, ys = []) => {
 const sum = {key1: 0, key2: 0}
 for (const obj of xs) {
 for (const item of obj.items) {
 sum.key1 += item.key1 * obj.num
 sum.key2 += item.key2 * obj.num
 }
 }
 for (const obj of ys) {
 sum.key1 += obj.gross * obj.num
 sum.key2 += obj.net * obj.num
 }
 return sum
}

Answer 2

I found it useful to ensure that the input data was sanitised to remove the possibility of undefined values before further processing. Much of the verbosity of the code was to do with checking for undefined when the alternate value was always 0, and that even extended to the computeVal function which became redundant.

The sample data was actually 'complete' and didn't include undefined values so I've not tested for outcomes based on imcomplete data. It might also be reasonable to replace || 0 with nullish (?? 0) and to consider if there might be strings or other types that would coerce to falsy and be converted to 0 when something else should happen.

Whilst it would be possible to refactor the nested reduce, it doesn't seem worth it as it now is easier to understand.

I also chose to abbreviate the variable names, again principally to reduce the verbosity without introducing ambiguity.

I suspect that the actual application data doesn't use opaque keys like key1 and key2, I suspect this was used to simplify the question. As a result, this might need a little more work to provide useful defaults, and generally the naming conventions might need aligning with the actual data.

function sanitiseObjects(objects) {
 return objects?.map(obj => {
 return {
 ...obj,
 num: obj.num || 0,
 items: obj.items.map(item => ({
 key1: item.key1 || 0,
 key2: item.key2 || 0,
 }))
 }
 }) || []
}
function sanitizeTotals(totals) {
 return totals?.map(total => ({
 ...total,
 num: total.num || 0,
 gross: total.gross || 0,
 net: total.net || 0,
 })) || []
}
function sumUpKeys(objects, totals = undefined) {
 const defaults = { key1: 0, key2: 0 }
 const totalsSum = sanitizeTotals(totals).reduce((acc, item) => {
 return {
 key1: acc.key1 + (item.num * item.gross),
 key2: acc.key2 + (item.num * item.net),
 };
 }, defaults);
 const objectsSum = sanitiseObjects(objects).reduce((objectsAcc, object) => {
 const itemsSum = object.items.reduce((itemsAcc, item) => {
 return {
 key1: (object.num * item.key1) + itemsAcc.key1,
 key2: (object.num * item.key2) + itemsAcc.key2,
 };
 }, defaults);
 return {
 key1: objectsAcc.key1 + itemsSum.key1,
 key2: objectsAcc.key2 + itemsSum.key2,
 };
 }, defaults);
 // add additional services sum to the monthly charges sum
 return {
 key1: objectsSum.key1 + (totalsSum?.key1 || 0),
 key2: objectsSum.key2 + (totalsSum?.key2 || 0),
 };
}
const objects1 = [
 { num: 3, items: [{ key1: 4, key2: 2, }, { key1: 10, key2: 20, }, ], key3: 'aa', },
 { num: 4, items: [{ key1: 4, key2: 2, }, ], key3: 'aa', },
];
const totals1 = [
 { num: 2, gross: 10, net: 5,},
 { num: 3, gross: 6, net: 12,},
];
// result with totals
const result1 = sumUpKeys(objects1, totals1);
// result without totals
const result2 = sumUpKeys(objects1);
console.log(result1);
console.log(result2);

Answer 3

I'm going to make an up-front assumption that many of the truthy checks you're doing (e.g. ... || 0) is simply to check if a value is undefined or not, and you don't actually expect other falsey values in your parameters. Using this assumption, I'll freely replace much of the || 0 logic with default parameters. If this assumption is wrong, you'll just have to toss the default parameters and add the falsey checks back in.

A major source of complexity here is the fact that each .reduce() is trying to update two independent values (key1 and key2). I found it to be much cleaner if I independently calculated the resulting key1 separately from the key2 value, instead of trying to calculate them both at the same time.

As an example, I changed the logic to calculate the totalSums object into this:

const totalsSum = {
 key1: totals.reduce((totalsAcc, currentTotalsItem) => (
 totalsAcc + computeVal(currentTotalsItem.num || 0, currentTotalsItem.gross)
 ), 0),
 key2: totals.reduce((totalsAcc, currentTotalsItem) => (
 totalsAcc + computeVal(currentTotalsItem.num || 0, currentTotalsItem.net)
 ), 0),
};

Notice how I have a different reduce function for each key? Splitting the code up like this let me notice other patterns that I could use to clean things up further. In the previous code snippet, I noticed that I could split up each .reduce() into two steps, a .map() step, then a .reduce() step.

const totalsSum = {
 key1: totals
 .map(({ num = 0, gross }) => computeVal(num, gross))
 .reduce((totalsAcc, x) => totalsAcc + x, 0),
 key2: totals
 .map(({ num = 0, net }) => computeVal(num, net))
 .reduce((totalsAcc, x) => totalsAcc + x, 0),
};

Those .reduce()es are now nothing more than a simple sum, so I could swap them out for a sum function.

const sum = array => array.reduce((acc, el) => acc + el, 0)
const totalsSum = {
 key1: sum(
 totals.map(({ num = 0, gross }) => computeVal(num, gross))
 ),
 key2: sum(
 totals.map(({ num = 0, net }) => computeVal(num, net))
 ),
};

I simplified the other areas of the code in a very similar fashion, and made some other simplifications beyond that, until I arrived at this fairly concise solution.

const sum = array => array.reduce((acc, el) => acc + el, 0);
const sumUpKeys = (objects, totals = []) => ({
 key1: sum([
 ...totals.map(({ num = 0, gross }) => num * gross),
 ...objects.map(({ num = 0, items }) => sum(
 items.map(({ key1 }) => num * key1)
 ))
 ]),
 key2: sum([
 ...totals.map(({ num = 0, net }) => num * net),
 ...objects.map(({ num = 0, items }) => sum(
 items.map(({ key2 }) => num * key2)
 ))
 ]),
});

From here, you'd notice there's a fair amount of repeated logic. If you think it makes sense, you could extract out a helper function that's capable of doing the sum for an individual key, then use that helper function for both keys. I opted not to do this, partly because I don't know enough about the concrete use case to know if this sort of refactor makes sense or not, but it is an option.

(In the end, I actually like @morbusg's final result better. But, I thought I would share the refactoring process I took, to hopefully give an idea of how to clean up code like this in the future).