6
\$\begingroup\$

I am working my way through FreeCodeCamp's Javascript challenges and Return Largest Numbers in Arrays took me a minute. In this one the goal was to create an array with the highest values from each array as its elements.

I am currently using for loops to iterate through the various arrays and arrays of arrays. I wanted to see if there was a better way to do this challenge.

function largestOfFour(arr) {
  // You can do this! 
  var largestNum = 0;
  var newArray = [];
  for(var i = 0; i < arr.length; i++){
    for(var k = 0; k < arr.length; k++){   
      console.log(arr[i][k]);
      //create comparison
      a = arr[i][k];
      if(a>largestNum){
        largestNum = a;
        newArray[i] = largestNum;
      }
    }
    largestNum=0;
  }
  console.log('complete Test' + newArray);
  return newArray;
}

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

\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

Bug

  for(var i = 0; i < arr.length; i++){
    for(var k = 0; k < arr.length; k++){   
      console.log(arr[i][k]);

There is a bug here.

  for (var i = 0; i < arr.length; i++) {
    for (var k = 0; k < arr[i].length; k++) {   
      console.log(arr[i][k]);

It only worked because both dimensions were the same. But that doesn't have to be true.

I would also prefer more whitespace to make it easier to read the code.

Nitpick

      console.log(arr[i][k]);

This looks like debugging code to me. You should remove debugging code before sending out for review (or going into production).

Avoid stomping globals

      a = arr[i][k];

If there is an a variable that is already visible in this scope, this will assign over that. That won't break anything in this code, but it could break other code, as its variables may change unexpectedly.

      var a = arr[i][k];

If you use var, it will create a new variable in this scope.

      if(a>largestNum){
        largestNum = a;

In this case, even if you don't switch to the @200_success solution, it would be even easier to just get rid of a.

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

Don't do extra work

You can simplify your interaction with largestNum:

function largestOfFour(arr) {
  var newArray = [];
  for (var i = 0; i < arr.length; i++) {
    var largestNum = 0;
    for (var k = 0; k < arr[i].length; k++) {   
      if (arr[i][k] > largestNum) {
        largestNum = arr[i][k];
        newArray[i] = largestNum;
      }
    }
  }

  return newArray;
}

This both reduces the scope and reduces the number of assignments. You clear largestNum one more time than is actually necessary.

Naming

/* Given a 2D array, return the maximum value from each row.  */
function findMaximums(arr) {
  var maximums = [];
  for (var i = 0; i < arr.length; i++) {
    maximums[i] = 0;
    for (var j = 0; j < arr[i].length; j++) {
      if (arr[i][j] > maximums[i]) {
        maximums[i] = arr[i][j];
      }
    }
  }

  return maximums;
}

I prefer a descriptive name like maximums to a generic name like newArray.

It is more common to use j as a second loop variable and k as the third.

I also prefer findMaximums to LargestOfFour, although that may be set by the challenge.

This seems like the simplest version of your original algorithm.

\$\endgroup\$
1
  • \$\begingroup\$ thanks. This is really helpful. good things to keep in mind as I grow \$\endgroup\$
    – Alex G
    Commented Jun 15, 2016 at 14:50
7
\$\begingroup\$

The challenge says that you will be given exactly four subarrays, but it doesn't specify the length of each subarray. You've assumed a square matrix, which is not stated in the problem.

To find the maximum of 4, 5, 1, and 3, you could write Math.max(4, 5, 1, 3). If those numbers are given to you as an array, you could use Math.max.apply(null, array).

To transform each element of the outer array, use arr.map().

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

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

\$\endgroup\$
6
  • \$\begingroup\$ Nice. You could even get rid of one more line if you use es6 syntax on the map callback :) \$\endgroup\$
    – Jonah
    Commented Jun 14, 2016 at 20:34
  • \$\begingroup\$ @Jonah ES6 and FreeCodeCamp don't always play nice together though. But yes, this could be re-writen like that. \$\endgroup\$
    – Mast
    Commented Jun 14, 2016 at 20:46
  • \$\begingroup\$ @200_success: what does the .apply() do to make that work? I have read the documentation and it still isn't clicking for me \$\endgroup\$
    – Alex G
    Commented Jun 14, 2016 at 21:18
  • \$\begingroup\$ @Jonah Care to explain that a little more? I am new to callback functions. \$\endgroup\$
    – Alex G
    Commented Jun 14, 2016 at 21:18
  • 1
    \$\begingroup\$ Does this explanation help? \$\endgroup\$ Commented Jun 14, 2016 at 21:21
1
\$\begingroup\$

Just to add to the answers above who have already said what i'd say, i just wanted to give you a few pointers how to help yourself along

Well, this is the solution i'd use...

function largestOfFour(arr) {
   var largest = [];
   for(var c=0;c<arr.length;c++) {
       largest.push(arr[c].sort(function(a,b){return a<b;}).shift());
   } 
   return largest;
}

I'd make more research in the native functions javascript provides instead of making your own functions for finding things. There really is no need to re-invent the wheel twice.

If you have chrome for example, assign a value to a variable, then type the variable and hit a .

Then see what google chrome autocompletes.

enter image description here

Then experiment with those functions and read up on them what they do. And you can come to those short solutions you've seen answered here.

\$\endgroup\$
2
  • \$\begingroup\$ Note that sorting the list is O(n log n), which is more work than the O(n) necessary to extract the maximum value of a list. \$\endgroup\$ Commented Jun 15, 2016 at 9:00
  • \$\begingroup\$ true, but that limitation wasn't stated to keep in mind the complexity. If we'd be iterating hundreds of thousands of numbers i'd use a different algorithem. but for the small samples provided i'd use this. \$\endgroup\$ Commented Jun 15, 2016 at 9:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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