7
\$\begingroup\$

Sometimes the best way to teach is show what you feel is a shiny approach instead of letting a student muddle through coding and potentially loose a future brilliant coder.

This is what I would show for 'Determine the largest averaged array`. Please review any and all aspects.

'use strict';
//Return the average of 1 array
function arrayAverage(array) {
  var length = array.length,
      sum = 0,
      i;
  for (i = 0; i < length; i++) {
    sum += array[i];
  }
  return sum / length;
}

//Compare 2 arrays, return the array with the largest average
//If the averages are equal, return array2
function largestAveragedArray(array1, array2) {

  return arrayAverage(array1) > arrayAverage(array2) ? array1 : array2;
}

//Figure out which array is largest, return `first` or `second` accordingly
function compare(array1, array2) {
  var largest = largestAveragedArray(array1, array2);
  return largest == array1 ? 'first' : 'second';
}

var smaller = [100, 80],
    larger = [100, 100];

document.write(compare(larger, smaller) + '<br>' + compare(smaller, larger));

\$\endgroup\$
1
  • 4
    \$\begingroup\$ Yay Stack Snippets! \$\endgroup\$ Sep 26 '14 at 20:16
6
\$\begingroup\$

First of all, a minor complaint, but be careful when using 'use strict'; at a file level—when concatenating JavaScript files for minification, this can have adverse affects. Whenever possible, use it at the top of a function instead.

Anyway, on to the actual code. Your arrayAverage function is fine, but it could be improved and made more explicit by using a more functional style. The .reduce function is perfect for this case, turning the function into a simple one-liner:

function arrayAverage(array) {
  return array.reduce(function (a, b) { return a + b; }) / array.length;
}

Your compare function is, frankly, rather odd. I see three main problems with it:

  • It does not handle arrays that have equal averages very well. Simply preferring the second array is not a good compromise.
  • Returning strings for the result is a little strange and unnatural. While there are no enums in a language like JavaScript, using string literals is not a good replacement.
  • The name compare is very generic and doesn't explain what the function does at all.

To fix all these problems, I propose the following function:

function arrayCompare(arrayA, arrayB) {
  var averageA = arrayAverage(arrayA),
      averageB = arrayAverage(arrayB);
  return averageA < averageB ? -1
       : averageA > averageB ?  1
                             :  0;
}

This is clearer, and it handles the equality case more elegantly. If you really hate nested conditional expressions with a passion, you could use a set of if statements instead, but I think this is perfectly readable when properly formatted.

This change eliminates the need for the largestAveragedArray function entirely, simplifying the code somewhat. As a final note, document.write makes me cringe, but I understand that it's useful in code snippets, so I'll let it slide here. Just don't use it in a real project.

(function () {
  'use strict';
  
  // returns the average of a single array
  function arrayAverage(array) {
    return array.reduce(function (a, b) { return a + b; }) / array.length;
  }
  
  // compares two arrays;
  // returns -1 if avg(a) < avg(b), 1 if avg(a) > avg(b), or 0 if avg(a) = avg(b)
  function arrayCompare(arrayA, arrayB) {
    var averageA = arrayAverage(arrayA),
        averageB = arrayAverage(arrayB);
    return averageA < averageB ? -1
         : averageA > averageB ?  1
                               :  0;
  }

  var smaller = [100, 80],
      larger = [100, 100];
  
  document.write(arrayCompare(smaller, larger) + '<br>' + arrayCompare(larger, smaller) + '<br>' + arrayCompare(smaller, smaller));
})();

\$\endgroup\$
4
  • 1
    \$\begingroup\$ Very good points, though I would leave functional programming to Coding 102 ;) Also what are your thoughts on handling equal averages ? \$\endgroup\$
    – konijn
    Sep 26 '14 at 20:07
  • 2
    \$\begingroup\$ @konijn I think they should be handled explicitly, as I did above. Admittedly, I cheated a bit—floating-point equality is trickier than that. If you wanted to handle equal averages correctly, you'd need to do some kind of fuzzy equality. Still, the result would be the same. \$\endgroup\$ Sep 26 '14 at 20:11
  • \$\begingroup\$ My bad, I missed that and I agree that is far more elegant \$\endgroup\$
    – konijn
    Sep 26 '14 at 20:14
  • \$\begingroup\$ Nice answer, neater code on a neat code base. However i would prefer, for the end of arrayCompare : ‘if (averageA==averageB) return 0;‘ ‘return (averageA<averageB) ? 1 : -1‘ \$\endgroup\$ Sep 29 '14 at 17:47
3
\$\begingroup\$

It contains a bug!

var smaller = [100, 80], larger = [];

The secound appears twice, because 0 / 0 is a NaN, and 90 > NaN is false.

length = array.length

This is worses the readability. In complicated expressions new variable could help, but array.length too simple to create a new variable.

function largestAveragedArray(array1, array2)

There is no reason to put comparing expression into a new method. KISS.

document.write(compare(larger, smaller) + '<br>' + compare(smaller, larger));

More fancy to use console.log(compare(larger, smaller), compare(smaller, larger)) ;) // console has many cool methods

And what about if equals?

\$\endgroup\$
2
  • \$\begingroup\$ Can you elaborate on unnecessary iterations? \$\endgroup\$
    – konijn
    Sep 26 '14 at 20:32
  • 1
    \$\begingroup\$ This still gets it wrong. The average of a list of no elements is not 0. Throw an exception, don't use a default if there isn't one. Also, your faster algorithm will fail if negative numbers are permitted. \$\endgroup\$ Sep 26 '14 at 22:01
-2
\$\begingroup\$

Actully thats a best algorithm for it

var smaller = [100, 80],
    same = [ 100,100],
      larger = [100, 100];


function largestAveragedArray( a, b ){
    var c = a.length > b.length ? a : b
        , i = 0
        , sumA = 0
        , sumB = 0
        , avgA = 0
        , avgB = 0;

    for( ; i < c.length; i++ ){
        sumA += a[ i ] ? a[ i ] : 0;
        sumB += b[ i ] ? b[ i ] : 0;
    }

    avgA = sumA / ( a.length > 0 ? a.length : 1 );
    avgB = sumB / ( b.length > 0 ? b.length : 1 );

    return avgA == avgB ? 0 : ( avgA > avgB ? 1 : -1 );
}

console.log( largestAveragedArray( smaller, larger ) );
console.log( largestAveragedArray( larger, smaller ) );
console.log( largestAveragedArray( same, larger ) );

It has O(n) complexity, being n the length of the biggest array. So instead of looping through both arrays it finds the biggest loop through it, sum a and b, acessing their indexes directly, and then divide the sum by it respectivo length to find the averages and then compare it.

Perfomance Test: http://jsperf.com/averageaarray

\$\endgroup\$
10
  • \$\begingroup\$ why is that Bruno? \$\endgroup\$
    – Malachi
    Sep 26 '14 at 21:29
  • \$\begingroup\$ It have O(n) complexity being 'n' the length of the biggest array. First find the biggest array then iterate through it saving the sum for the first and the second array provided, and then i compare them. \$\endgroup\$ Sep 26 '14 at 21:37
  • 1
    \$\begingroup\$ would you mind incorporating that into your answer? it would make your answer much better and more likely to get upvotes. \$\endgroup\$
    – Malachi
    Sep 26 '14 at 21:38
  • \$\begingroup\$ Both your solution and the original solution have the same complexity, O(m*N), m being the number of arrays to compare. If anything, the original solution is better because it is trivial to add a third, fourth, or fifth array to the comparisons. Your solution would require quite the rewrite! \$\endgroup\$ Sep 26 '14 at 22:27
  • \$\begingroup\$ You are wrong, mines does not iterates through both arrays, instead it access them directly by índex, for more arrays you just need to find the biggest of all to iterate \$\endgroup\$ Sep 26 '14 at 23:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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