HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Finding the largest averaged array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thearrayaveragedlargestfinding

Problem

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 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) + '
' + compare(smaller, larger));`

Solution

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
                             :  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), or 0 if avg(a) = avg(b)
function arrayCompare(arrayA, arrayB) {
var averageA = arrayAverage(arrayA),
averageB = arrayAverage(arrayB);
return averageA averageB ? 1
: 0;
}

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

document.write(arrayCompare(smaller, larger) + '
' + arrayCompare(larger, smaller) + '
' + arrayCompare(smaller, smaller));
})();

Code Snippets

function arrayAverage(array) {
  return array.reduce(function (a, b) { return a + b; }) / array.length;
}
function arrayCompare(arrayA, arrayB) {
  var averageA = arrayAverage(arrayA),
      averageB = arrayAverage(arrayB);
  return averageA < averageB ? -1
       : averageA > averageB ?  1
                             :  0;
}

Context

StackExchange Code Review Q#63981, answer score: 6

Revisions (0)

No revisions yet.