Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have implemented the "Sum All Numbers in a Range" challenge from Free Code Camp quoted below.

The code works like a charm, but I don't think it's idiomatic. The challenge hints imply I should be using Math.max(), Math.min() and Array.reduce(). In addition I used the spread operator from ES6 to simplify both Math calls.

I'm looking for a more idiomatic way to solve this. I think it can be done in a one-liner, something like:

return arr.sort((function(a, b) { return a - b; })).reduce(function(a, b) { /* */ });

But I'm pretty sure I'm heading the wrong way with the above.

The challenge:

We'll pass you an array of two numbers. Return the sum of those two numbers and all numbers between them.

The lowest number will not always come first.

The code:

function sumAll(arr) {
  var out = 0;
  for (var i = Math.min(...arr); i <= Math.max(...arr); i++) {
    out += i;
  }
  return out;
}

Test cases:

sumAll([1, 4])
sumAll([4, 1])
sumAll([5, 10])
sumAll([10, 5])

Expected output:

10
10
45
45
share|improve this question
    
How do you get freecodecamp to use es6? – Barry Jan 18 at 16:56
    
@Barry It's not allowed for every challenge for some reason, but on this one it just gave a notification ES6 was required. Nothing else. – Mast Jan 18 at 17:11
1  
ever heard of Gauss? (en.wikipedia.org/wiki/Carl_Friedrich_Gauss#Anecdotes) – njzk2 Jan 18 at 21:53
1  
the oneliner you give is very close. in the final function you have a and b being the min and max values. just apply gauss to it: return (a+b)*(b-a+1)/2; – njzk2 Jan 18 at 21:58
up vote 14 down vote accepted

I don't know what you would use reduce() for to be honest. But you don't need to loop. We just have a simple arithmetic sequence with a straightforward formula:

$$\sum_{i=min}^{max} i = \frac{(max-min+1)(min+max)}2$$

We just need to pick out which one is larger:

function sumFrom(min, max) {
    return (max-min+1) * (min+max) / 2;
}

function sumAll(arr) {
    return sumFrom(Math.min(...arr), Math.max(...arr));
}

Or we could generalize to any arithmetic sequence:

function sumArithmetic(a1, n, d) {
    return n*(2*a1 + (n-1)*d) / 2;
}

function sumAll(arr) {
    var min = Math.min(...arr);
    return sumArithmetic(min, Math.max(...arr) - min + 1, 1);
}

I guess if you really want reduce() you'd do it this way (not using ES6 because I don't know how to get freecodecamp to use it):

function sumAll(arr) {
  var min = Math.min.apply(null, arr);
  var max = Math.max.apply(null, arr);

  return Array.apply(null, Array(max-min+1))   // get array of correct size
      .map(function(_, b) { return b+min; })   // change it to have correct values
      .reduce(function(a, b) { return a+b; }); // and sum it
}
share|improve this answer
    
you would use reduce because it is a way to not access the array by indexes. – njzk2 Jan 18 at 21:54
    
$max(a,b)-min(a,b)=|a-b|$, $min+max=a+b$ saves you some steps. – A.S. Jan 19 at 1:09

Like Barry mentioned, you are probably expected to use Gauss and the property of triangular numbers.

reduce is a funny way of accessing your array without touching the indexes, hence without needing to verify that they exist. (consider what would happen if your function was called with [] or [0]).

The one-liner you give is very close. In the final function you have a and b being the min and max values in that order. just apply the sum formula to it: return (a+b)*(b-a+1)/2;

So:

return arr
        .sort((a, b) => a - b)
        .reduce((a, b) => (a + b) * (b - a + 1) / 2);

You'll notice that since the array is size 2 and known to be that way, the complexity is O(1), while if you actually sum the numbers, it becomes O(n) where n is the amount of numbers to add.

share|improve this answer

Personally, I'd change out to result. And also the structure in something like the following:

function sumAll(arr){
    var smallest = 0;
    var greatest = 0;

    if(arr[0] < arr[1]){
        smallest = arr[0];
        greatest = arr[1];
    } else{
        smallest = arr[1];
        greatest = arr[0];
    }

    return sumRange(smallest, greatest);
}

function sumRange(from, to){
    var result = 0;

    for(var i = from; i <= to; i++){
        result += i;
    }

    return result;
}

In this way it becomes more readable IMO.

share|improve this answer
1  
Javascript does not support ad hoc polymorphism. only the second function exists. – njzk2 Jan 18 at 22:09
    
@njzk2 thanks for pointing that out. Silly mistake. Now it should be ok. – Gentian Kasa Jan 18 at 22:48

This is my solution using Math.max and Math.min. I stored the max and min of the array in variables, then looped through the array, pushing the numbers and those in between (using i) to an empty array. I then reduced the empty array.

function sumAll(arr) {
    var allNumArray = [];
    var max = Math.max.apply(null, arr);
    var min = Math.min.apply(null, arr);
    for (var i = max; i >= min; i--) {
        allNumArray.push(i);
    }
    return allNumArray.reduce(function(a, b) {
        return a + b
    });
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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