Is there a way to make this Javascript code more efficient?

Question

It is a simple exercise that I am doing for mere practice and leisure, I have done it in various ways but I was wondering if there is an even more practical way or to reduce the lines of code making use of the many methods of JavaScript.

The exercise is about receiving an array (arr) and a number (target) and returning another array with a pair of numbers found in 'arr' whose sum is equal to 'target'.

function targetSum3(arr, target) { let newArr = []; let copyArray = arr; for (let i of copyArray) { let x = Math.abs(i - target); copyArray.pop(copyArray[i]); if (copyArray.includes(x) && (copyArray.indexOf(x) != copyArray.indexOf(i))) { newArr.push(i); newArr.push(x); return newArr; } } return newArr; }

Why are you giving Array.pop() method an argument? Afaik it is obsolete, isn't it? — fonzane
– fonzane, Commented Nov 22, 2021 at 12:19
This line doesn't make much sense to me: copyArray.indexOf(x) != copyArray.indexOf(i). Since x and i will only be different values when target is 0, this will only return false when target is 0??? — fonzane
– fonzane, Commented Nov 22, 2021 at 12:30
The line copyArray.indexOf(x) != copyArray.indexOf(i). is because in that way, if I have for example arr = [1,2,3] and target = 6. It does not return me [3,3] since the number 3 appears just one time. — Nahuel Valles
– Nahuel Valles, Commented Nov 22, 2021 at 12:38

fonzane · Accepted Answer · 2021-11-22 13:22:06Z

1

If you are fine with a function that just returns a pair of numbers (the first match so to speak) whose sum equals the targets value, this might be enough:

function sumPair (arr, target) { while(arr.length) { let sum1 = arr.shift(); let sum2 = arr.find(val => sum1 + val === target); if (sum2) return [sum2, sum1]; } return null; }

answered Nov 22, 2021 at 13:22

fonzane

4602 gold badges8 silver badges22 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

KooiInc Over a year ago

Note: this will mutate the original Array. If no sum is found, it will be empty.

Nahuel Valles Over a year ago

Could you explain to me why the while(arr.length) does not enter in a infinite loop?

fonzane Over a year ago

It's because of the line let sum1 = arr.shift(); arr.shift() removes the first element of the array and returns it. So sum1 will be equal to the first element and the array itself will be shortened by one whenever this line gets executed (arr.length -1). If sum2 never gets found, arr.length will at some point be 0 which coerces to false in a boolean check.

A1exandr Belan · Accepted Answer · 2021-11-22 13:47:38Z

const targetSum = (arr, target) => { const first = arr.find((v,i,a) => arr.includes(target-v) && (arr.indexOf(target-v) !== i)); return first ? [first, target - first] : null; }; const values = [1,2,3,4,5,6,7,8,9]; console.log(targetSum(values, 1)); // null console.log(targetSum(values, 2)); // null console.log(targetSum(values, 3)); // [1, 2] console.log(targetSum(values, 15)); // [6, 9] console.log(targetSum(values, 20)); // null

fonzane · Accepted Answer · 2021-11-22 13:01:08Z

I changed for loop with forEach (more efficient) and there is no need for the copyArray array so I removed it. I also changed pop() with shift(), I think you want to shift the array and not pop-it (if I understand the task correctly).

function targetSum3(arr, target) { let newArr = []; arr.forEach(element => { let x = Math.abs(element - target); // calc x arr.shift(); // removes first element from arr (current element) if (arr.includes(x) && (arr.indexOf(x) != arr.indexOf(element))) { newArr.push(element); newArr.push(x); return; } }); return newArr; }

I did try with the for each, but when you run it, it says 'Ilegal break statement' that's why I changed it for a for of, any idea of why that happens? I just tried yours and I got the same problem...
You can't use break; in a forEach function. Use return; instead. break only works in for loops. But also this function is faulty. If you give it [3,4,1] and 2 as target, it will return [3,1] but those numbers don't equal 2 if added together. There is a logical flaw in let x = Math.abs(element-target)
Yes, my bad. Forgot that you can't break a forEach, I'll fix my answer.

KooiInc · Accepted Answer · 2021-11-22 13:28:28Z

use Array.filter to find the target sum for all values in an given array. See comments in the snippet.

sumsForTargetInArray(); document.addEventListener(`click`, evt => evt.target.id === `redo` && sumsForTargetInArray()); function sumsInArray(arr, target) { // clone the array const clone = arr.slice(); let result = []; while (clone.length) { // retrieve the current value (shifting it from the clone) const current = clone.shift(); // filter arr: all values where value + sum = target const isTarget = arr.filter(v => current + v === target); // add to result. // Sorting is to prevent duplicates later if (isTarget.length) { result = [...result, ...isTarget.map(v => [current, v].sort())]; } } // weed out duplicates (e.g. 0 + 3, 3 + 0) const unique = new Set(); result.forEach(r => unique.add(`${r[0]},${r[1]}`)); // return array of array(2) return [...unique].map(v => v.split(`,`).map(Number)); } function sumsForTargetInArray() { const testArr = [...Array(20)].map((_, i) => i); const target = Math.floor(Math.random() * 30); document.querySelector(`pre`).textContent = `testArray: ${ JSON.stringify(testArr)}\ntarget: ${target}\nResult: ${ JSON.stringify(sumsInArray(testArr, target))}`; }

<pre></pre> <button id="redo">Again</button>

Collectives™ on Stack Overflow

Is there a way to make this Javascript code more efficient?

4 Answers 4

3 Comments

Comments

3 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

3 Comments

Comments

Related