Return Largest Numbers in Arrays - improve my code

Hi All,
this challene toke me 4 hours :slight_smile: .
I just want some suggestion on how to improve my code. or reviews
thank you.

function largestOfFour(arr) {
  // You can do this!
  var numL =0;
  result = new Array ();
  
  
  for (var i=0; i < arr.length; i++) {
      
    numL =0;
        for (var j=0; j <4; j++)  {
            if (numL < arr[i][j] )
                      numL=arr[i][j];        
                 
        } 
        result.push(numL);
    }
         
  return result;
}


When saw your post I taught the obvious answer is :

function largestOfFour(arr) {
  return arr.map(function(elt) {
    return elt.reduce(function(acc, cur) {
      return Math.max(acc, cur);
    });
  });
}

Looking at the solution I had done at the time and well it was quite similar to what you did (except I used a “findMax” function just to make the code more readable).
So, my advice would be for you to look at all the methods that exist to perform operation on arrays : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array
Here I used map and reduce, all those methods are VERY handy.

Using your existing code, I only see 1 improvement and 2 adjustments you could do to shrink the code more.

  1. remove the first var numL = 0; (line 3) because you are only going to use it in your nested for loop.
  2. remove result = new Array (); (line 4) and put result = [] into your outer for loop declaration (see below):
for (var i=0, result=[]; i < arr.length; i++) {
  1. move the numL =0; into your inner/nested for loop declaration (see below):
for (var j=0, numL=0; j <4; j++)  {

When put all together, you will have:

function largestOfFour(arr) {
  // You can do this!
  for (var i=0, result=[]; i < arr.length; i++) {
    for (var numL =0,j=0; j <4; j++)  {
      if (numL < arr[i][j] )
        numL=arr[i][j];        
    } 
    result.push(numL);
  }
  return result;
}

I believe you did not get the idea of how array works. So, do you use two-dimensional array or one-dimensional array? If you use one-dimensional array, then you have this code:

function largestOfFour(arr) {
  result = arr[0];
  for (var i=1; i < arr.length; i++) {
      
            if (arr[i] > result )
                      result = arr[i];         
    }
  return result;
}

thank you for feedback,
yes , i used two dimensional array to get the largest numbers.

If I would do this challenge then I prefer this way.

function largestOfFour(arr) {
  var myArr = arr.sort(function(a, b) { return (b - a); });
  return myArr[0];
}
largestOfFour([2, 5, 7, 9]);

In this code I’ve just sorted out array(arr) from highest to lowest and then put it into my new Array(newArr). Because I’ve sorted it from highest to lowest, then i know first number of my new array which will be on newArr[0] will be the highest.

For more details about sort visit this link: https://www.w3schools.com/jsref/jsref_sort.asp

Hope it helped you.

Happy Coding! :slight_smile:

@Nurlyssultan - Your solution would not work, because the arr parameter in the function is an array of arrays and not just a single array.

This is another way:

function largestOfFour(arr) {
    return arr.reduce(function (acc, num) {
      return Math.max(acc, num);
    });
}

This would also work:

function largestOfFour(arr) {
    return Math.max.apply(null, arr);
}

Using the spread operator, we could also do it like this:

function largestOfFour(arr) {
    return Math.max(...arr);
}

There’s more than one way to skin a cat.
.

1 Like

The best improve you can get should come from, that’s why you need to learn as much as you can in my opinion. That way you could also reduce some painful errors. Of course you can use some programs like checkmarx or others to detect errors but it’s also recommended to do it on your own.
Good luck.
Michael.

Your code works and I don’t really see any issues with how you’ve approached the problem. I think you could definitely shorten it if you wanted to, using JS’s map and Math.max functions. I’ll write it out in ES6:

let getLargest = (arr) => arr.map( 
            sub => Math.max.apply(null, sub)) 

However, this is just as effective/fast as your code, and maybe a bit more unreadable so it’s just a fun exercise, and not much of an improvement.

I really like the idea of using this kind of method for the challenge. However, since there are multiple arrays involved – would you have to split and then join the 4 greatest numbers of each array?