Eloquent Javascript exercise: find average age from ancestors array per century I solved an exercise from the book "Eloquent Javascript". I want to share with you guys the solution. The exercise is from chapter 5. Summing up, I have to find the average age from the ancestors array per century. Here is my solution: 

var ancestry = JSON.parse(require('../resources/ancestry'))

//Top-down approach, functions are defined after their use
//Necessary side-effect in function to print the output
function printAverageAgePerCentury(ancestry){
    var averageAgePerCentury = getAverageAgePerCentury(ancestry);
    Object.keys(averageAgePerCentury).forEach(function(century){
        console.log(century.concat(": ").concat(averageAgePerCentury[century]))
    })
}

function getAverageAgePerCentury(ancestry){
    var agesPerCentury = getAgesPerCentury(ancestry); 
    return agesPerCentury.reduce(function(averageAgePerCentury, agePerCentury){
        if(averageAgePerCentury[agePerCentury.century]){
            averageAgePerCentury[agePerCentury.century] = [average(averageAgePerCentury[agePerCentury.century].concat(agePerCentury.age))];
            return averageAgePerCentury;
        } else {
            averageAgePerCentury[agePerCentury.century] = [agePerCentury.age];
            return averageAgePerCentury;
        }
    }, {})
}

function getAgesPerCentury(ancestry){
    return ancestry.map(function(person){
        return {
            century : whichCentury(person),
            age : person.died - person.born
        }
    })
}

function average(array){
    function plus(a,b){ return a + b}
    return array.reduce(plus) / array.length
}

function whichCentury(person){
    return Math.ceil(person.died / 100)
}

//--------------------------------//

printAverageAgePerCentury(ancestry);



What do you think about the top-down approach? Do you prefer the opposite? Where functions are defined before their use.
Is possible to make this solution more readable, more elegant?
What could you do to make it better?
Incorrect calculation of average

average([1, 2, 3, 4]) is \$\dfrac{1+2+3+4}{4}=2.5\$.

However, due to the way you call reduce() in getAverageAgePerCentury():


averageAgePerCentury[agePerCentury.century] = [average(averageAgePerCentury[agePerCentury.century].concat(agePerCentury.age))];



… you would actually calculate average([average([average([average([1]), 2]), 3]), 4]), which would produce a result of 3.125 — weighted in favour of the later entries.

Doing too much

A root cause of the bug, I think, is that getAverageAgePerCentury() is doing a lot of work.  It creates a map of centuries to the age of people who died in that century, and calculates the average of those values.

I would define two utility functions, inspired by Lodash's _.mapValues() and _.groupBy().

/**
 * Creates a new object in which all of the original obj's values are
 * transformed by callback(value).
 */
function mapValues(obj, callback) {
    return Object.keys(obj).reduce(function(result, key) {
        result[key] = callback(obj[key]);
        return result;
    }, {});
}

/**
 * Given an array of objects, creates a new object in which all objects where
 * attrExtractor(obj) having the same value are grouped together in an array.
 */
function groupBy(objs, attrExtractor) {
    return objs.reduce(function(result, obj) {
        var attr = attrExtractor(obj);
        if (attr in result) {
            result[attr].push(obj);
        } else {
            result[attr] = [obj];
        }
        return result;
    }, {});
}


Then, the rest of the solution looks simpler.

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

function averageAgePerCentury(ancestry) {
    function deathCentury(person) { return Math.ceil(person.died / 100) }
    function deathAge(person) { return person.died - person.born }

    return  mapValues(
                mapValues(
                    groupBy(ancestry, deathCentury),
                    function(persons) { return persons.map(deathAge) }
                ),
                average
            );
}


Note that I've renamed getAverageAgePerCentury().  A "getter" function should retrieve something that already exists.  Here, you're doing a calculation.  You call Math.sin(x), not Math.getSin(x), so the same naming principle applies here.