29

I have a task I found on CodeWars and I managed to solve it, however, after submitting is says:

Execution timed out: (12000 ms)

When I try to test the function is passed, but I guess it is too slow. Before you condemn me for not finding the answer on my own. I don't really care about submitting that as a response, but I have no idea how to make it faster and that is why I am here. Here is the function:

const ls = [0, 1, 3, 6, 10]

const partsSums = (ls) => {
    const sum = []
    for(let i = 0, len = ls.length; i < len + 1; i++) {
        let result = ls.slice(i).reduce( (accumulator, currentValue) => accumulator + currentValue, 0)
        sum.push(result)
    }
    return sum
}

Here are the instructions:

Let us consider this example (array written in general format):

ls = [0, 1, 3, 6, 10]

Its following parts:

ls = [0, 1, 3, 6, 10]
ls = [1, 3, 6, 10]
ls = [3, 6, 10]
ls = [6, 10]
ls = [10]
ls = []

The corresponding sums are (put together in a list): [20, 20, 19, 16, 10, 0]

The function parts_sums (or its variants in other languages) will take as parameter a list ls and return a list of the sums of its parts as defined above.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Cortoloman
  • 695
  • 1
  • 7
  • 14
  • 4
    actually you need to go here https://codereview.stackexchange.com/ – bill.gates Aug 04 '20 at 07:11
  • 2
    @Ifaruki I disagree. The CR is for code that *works* but can be made better. This one *doesn't* work. It fails in the execution time, which means it doesn't fulfil the requirements. It has problem that needs to be fixed which is what SO is about. – VLAZ Aug 04 '20 at 07:14
  • 9
    @VLAZ: That's ridiculous. The code works, if the OP is to be believed. It just needs to be improved, not fixed. That is what code review is about. – TonyK Aug 04 '20 at 20:48
  • 1
    Can you put the link to the Codewars kata? – haxor Aug 04 '20 at 21:51
  • 1
    @haxor It's part of my [answer](https://stackoverflow.com/a/63256004/3231537), you can find the kata [here](https://www.codewars.com/kata/5ce399e0047a45001c853c2b/train/javascript) – Icepickle Aug 05 '20 at 00:05
  • 3
    @VLAZ Code Review even has a [time-limit-exceeded tag](https://codereview.stackexchange.com/questions/tagged/time-limit-exceeded). – Solomon Ucko Aug 05 '20 at 01:30
  • @TonyK yes the code did work, it passed the initial test but it wasn't accepted because it was too slow. I just posted it here because I didn't know about code review. And a big thanks to everyone. I learned a lot from all these comments! – Cortoloman Aug 05 '20 at 14:55

6 Answers6

18

For this kind of array maipulations, you better not use build in methods, like slice or reduce, because they are slow in comparison to a for loop, or any other looping approaches.

This approach takes a sinlge loop and uses the index for getting a value of the given array and takes the last sum of the new array.

Some speed tests on Codewars: Sums of Parts:

  • 5621 ms with sparse array sum = []; sum[i] = 0; (the first version of this answer),
  • 3452 ms with Array(i + 1).fill(0) and without sum[i] = 0;,
  • 1261 ms with Array(i + 1) and sum[i] = 0; (find below),
  • 3733 ms with Icepickle's first attempt.

const
    partsSums = (ls) => {
        let i = ls.length;
        const sum = Array(i + 1);

        sum[i] = 0;
        while (i--) sum[i] = sum[i + 1] + ls[i];

        return sum;
    },
    ls = [0, 1, 3, 6, 10];

console.log(...partsSums(ls));
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392
  • 14
    `slice` and `reduce` are not slow in comparison to looping explicitly, the problem with the OP's code is the `O(n²)` algorithm. – Bergi Aug 04 '20 at 18:50
  • Any idea why [mine](https://stackoverflow.com/a/63256004/3231537) seems to be faster? Is it the array index access, it doesn't really seem to make much sense... On a second look, if I prefill the `sum` array beforehand, the execution time is a lot faster as well – Icepickle Aug 04 '20 at 22:51
  • 2
    @Icepickle Yes, Nina's answer fills the array from the end, creating a sparse array that's probably not well optimised. Growing it from the front is generally faster. – Bergi Aug 04 '20 at 23:34
  • now with an array of the wanted length. this should work faster now. – Nina Scholz Aug 05 '20 at 05:59
  • When I run Cortoloman's function from the original question it takes 1129 ms - which is faster than this answer! – James Aug 24 '20 at 12:16
  • @James, do you have a link to the referenced answer? – Nina Scholz Aug 24 '20 at 13:15
  • @NinaScholz I just opened the link from your answer, pasted in the code from the question and clicked Test. Maybe we are just measuring how busy the CodeWars server is! – James Aug 24 '20 at 13:37
  • @James, but op stated `12000 ms`. is this value not comparable? – Nina Scholz Aug 24 '20 at 13:39
  • @NinaScholz It is not comparable - look at the number of digits! The op said 12000 ms. Either the op was wrong or it is now running about 10 times faster. – James Aug 24 '20 at 13:45
  • @James, op has some more iterations than my code. `slice` is expensive and it makes no sense that op's code is faster than mine. – Nina Scholz Aug 24 '20 at 13:47
  • @NinaScholz when it comes to optimisation you should always measure and never make assumptions. JavaScript has complex optimisation and just in time compilation which makes it hard to predict how it will perform. It might be taking longer to upload and interpret the code than to run it. Speed has improved as V8 optimisation has improved over time. Why do you assume `slice` is slow? – James Aug 24 '20 at 13:58
11

You can still take a more functional approach but optimise the way you're doing the calculations.

Here is the idea - since you're trying to sum all items, then sum all but the first, then sum all but the second, etc., mathematically equivalent to getting the sum then subtracting from it each number in order and keeping the total.

[sum([41, 42, 43]), sum([42, 43]), sum([43]), sum([])]

is the same as:

total = sum([41, 42, 43])
[total - 0, total - 0 - 41, total - 0 - 41 - 42, total - 0 - 41 - 42- 43]

is the same as:

total = sum([41, 42, 43])
[total -= 0, total -= 41, total -= 42, total -= 43]

Generalised, this looks like:

total = sum([a1, a2, ..., aN])
[total -= 0, total -= a1, total -= a2, ..., total -= aN]

Using the trusty Array#reduce we can derive the sum once. Then we can derive the new array using Array.map using ls.map(num => total -= num).

The only problem here is that we get one less item - we don't calculate the initial total -= 0 which has to exist for all items. One way to do it is to append it to the start [0].concat(ls) will create the correct array to map over. However, since we already know what the value there would be, we can skip this step and directly substitute with total (after all the result of total -= 0 is total and leaves total unchanged). So, we can directly use [total].concat(ls.map(num => total -= num)) to start with total and add the rest of the items. to the end.

const ls = [0, 1, 3, 6, 10]

const partsSums = (ls) => {
    let total = ls.reduce((a, b) => a + b, 0);
    
    return [total]
      .concat(
        ls.map(num => total -= num)
      );
}

console.log(partsSums(ls));
VLAZ
  • 26,331
  • 9
  • 49
  • 67
  • 2
    I misread that as `num => total - num`. *If* you have to use a side effect (not a very functional approach) in `map`, better make it explicit: `num => { total -= num; return total; }` – Bergi Aug 04 '20 at 18:53
4

Personally, I would just use the previous sum to calculate the next, I don't see any need to re-iterate all the previous sums, so, I would probably go for a basic loop and then reverse the results, like so

function partsSums(ls) {
  const result = [0];
  if (ls.length === 0) {
    return result;
  }
  for (let i = ls.length, q = 0; i--; q++) {
    result.push(result[q] + ls[i]);
  }
  return result.reverse();
}

or, without reversing, look more like Nina's solution (except for predefining the length of the array)

function partsSums(ls) {
  const len = ls.length;
  const result = new Array(len+1);
  result[len] = 0;
  for (let i = len; i--;) {
    result[i] = result[i+1] + ls[i];
  }  
  return result;
}

Both also seem to run faster than Nina's on codewars nodejs engine, in the first part probably because of push, in the second one, probably because the array's length is defined from the start, for more information see this question

Icepickle
  • 12,689
  • 3
  • 34
  • 48
1

A solution using normal for loop along the time of execution .

var arr = [0, 1, 3, 6, 10];


function giveList(array){
    
    var sum=0;
    for(let i=0;i<array.length;i++){
       sum=sum+array[i];
    }

    var result = [];
    result.push(sum);
    var temp;
    for(let i=0;i<array.length;i++){
       temp=sum-array[i];
       result.push(temp); 
       sum=sum-array[i];
        
     }
 return result;
}

console.time();
console.log(giveList(arr));
console.timeEnd();
1
const partsSums = (ls, sum = 0) =>
   [...ls, 0].reverse().map(x => sum = x + sum).reverse();

Takes around 1100 ms when I run it on CodeWars, which is slightly faster than other answers.

James
  • 5,635
  • 2
  • 33
  • 44
0

The repeated operation is too more. e.g: when you compute sum of [3, 6, 10], the up step [1, 3, 6, 10] already compute。 So you can think in another direction, back to end compute the sum of array

const ls = [0, 1, 3, 6, 10];

function partSums(ls) {
   const len = ls.length;
   const dp = [];

   if(len === 0) { return [0] }
   dp[len] = 0;
   dp[len - 1] = ls[len - 1];
   for (let i = len - 2; i >= 0; i--) {
     dp[i] = dp[i + 1] + ls[i];
   }

   return dp;
}