Creating an array consisting of the largest values of each sub-array does not work as expected

Find the largest number in each of the sub-array and then make an array of those largest numbers.[[4, 5, 1, 3], [13, 27, 18, 26], [32, 35, 37, 39], [1000, 1001, 857, 1]]

I wrote some code, and I can't figure out what is wrong with it. Maybe the Array.push() method doesn't work or perhaps the for loops.

function largestOfFour(arr) {
    var main = [];
    for(k=0;k<arr.length;k++){
       var long= 0;
         for(i=0;i<arr[k].length;i++){
            if(arr[k][i]<long) {
                arr[k][i] = long;
            }
            main.push[long];
        }
    }
    return main
}

largestOfFour([[4, 5, 1, 3], [13, 27, 18, 26], [32, 35, 37, 39], [1000, 1001, 857, 1]], "");

Solution 1:

The problem is on the inner loop, when you try to find the max value for each array. On each iteration of the outer loop, you should reset long = arr[k][0]. It should not be reset to 0 since the max value may be smaller than 0. Note that this expects all subarrays to have at least one item.

As noted by @edc65, the declaration of long should occur at the start of the function to make it clear that long, as all local variables, has a function scope.

You only want one value per subarray. Therefore you should be adding one value for each iteration of the outer loop (main.push should be in the outer loop). The way it is currently, you are adding one value per subarray element.

In the if statement, your assignment is inverted. It should be

long = arr[k][i];

And the condition is also inverted. long stores the max value for each subarray. Therefore, you update it if you find a value greater than it:

if(arr[k][i]>long) {
    long = arr[k][i];
}

When pushing into the array use parenthesis, not brackets:

main.push(long);

Parenthesis are for calling methods. Brackets are for accessing properties in an object.

Final code

function largestOfFour(arr) {
    var main = [];
    var long;
    for(k=0;k<arr.length;k++){
       long = arr[k][0];
         for(i=0;i<arr[k].length;i++){
            if(arr[k][i]>long) {
                long = arr[k][i];
            }
        }
        main.push(long);
    }
    return main;
}

Math.max method

You can use Math.max to simplify your code

function largestOfFour(arr) {
    var main = [];
    for(k=0;k<arr.length;k++){
        var long = Math.max.apply(null, arr[k]);
        main.push(long);
    }
    return main;
}

As per @BillyMoon's and @Tushar's answers, this can be further simplified to an Array.map call.

Solution 2:

I know the question here is to find a bug in the existing code, in case if you may want to optimise the code

The original idea is of @thefourtheye. I'm just explaining this here.

No need of nested loops, you can achieve this in single line.

var arr = [[4, 5, 1, 3], [13, 27, 18, 26], [32, 35, 37, 39], [1000, 1001, 857, 1]];

var result = arr.map(Math.max.apply.bind(Math.max, null));

document.write(result);
console.log(result);

How this works?

The array.map function is iterating over each of the elements from array on which it is called. The function passed to the map here is apply with its this context bound to the Math.max and first argument bound to null.

Math.max.apply.bind(Math.max, null) this basically calls the Math.max function on array as

Math.max.apply(null, array);

Update:

With ES6, arrow function and spread operator, this can be made even smaller

arr.map(e => Math.max(...e))

var arr = [[4, 5, 1, 3], [13, 27, 18, 26], [32, 35, 37, 39], [1000, 1001, 857, 1]];

var result = arr.map(e => Math.max(...e));

document.write(result);
console.log(result);