Finding the mode of an array

Question

I am a beginner in JavaScript and just wanted to get my code reviewed. Here I have created a function that takes in an array and returns the mode. If there is no more then I just want the function to return the first number. It seems a little too long and I feel like there may be more clever ways to do it. Again, keep in mind that I am a beginner, so I don't have a huge toolbox.

var mode = function(arr){
    var numMapping = {};
    for(var i = 0; i < arr.length; i++){
        if(numMapping[arr[i]] === undefined){
            numMapping[arr[i]] = 0;
        }        
            numMapping[arr[i]] += 1;
    }
    var greatestFreq = 0;
    var mode;
    for(var prop in numMapping){
        if(numMapping[prop] > greatestFreq){
            greatestFreq = numMapping[prop];
            mode = prop;
        }
    }
    return parseInt(mode);
}

Answer 1

var mode = function(arr){

It's nice to name your functions for debugging purposes, if none other. You could replace this with a function declaration, e.g. function mode(arr) { or just name the function expression: var mode = function mode(arr) {

    var numMapping = {};
    for(var i = 0; i < arr.length; i++){
        if(numMapping[arr[i]] === undefined){
            numMapping[arr[i]] = 0;
        }        
            numMapping[arr[i]] += 1;
    }

I would use arr.forEach(), since it lets you operate on the elements directly. You could also make the mapping a one-liner(but both of these things are more nitpicks/personal style choice)

    var greatestFreq = 0;
    var mode;
    for(var prop in numMapping){
        if(numMapping[prop] > greatestFreq){
            greatestFreq = numMapping[prop];
            mode = prop;
        }
    }
    return parseInt(mode);
}

You can do this whole part during your first loop! Also, parseInt, while safe in this case, is not generally safe to use without a radix(you'd have linters complaining about this usage here). You could use parseInt(mode, 10), or some ToNumber() transformation like +mode(http://es5.github.io/#x11.4.6).

Here's a revised example:

var mode = function mode(arr) {
    var numMapping = {};
    var greatestFreq = 0;
    var mode;
    arr.forEach(function findMode(number) {
        numMapping[number] = (numMapping[number] || 0) + 1;

        if (greatestFreq < numMapping[number]) {
            greatestFreq = numMapping[number];
            mode = number;
        }
    });
    return +mode;
}

Answer 2

As others have said, this is reasonable beginner's code. And @Sacho's answer gives good suggestions for improving it.

Below is a solution using some slightly more advanced techniques, if you're interested:

var mode = function mode(arr) {
    return arr.reduce(function(current, item) {
        var val = current.numMapping[item] = (current.numMapping[item] || 0) + 1;
        if (val > current.greatestFreq) {
            current.greatestFreq = val;
            current.mode = item;
        }
        return current;
    }, {mode: null, greatestFreq: -Infinity, numMapping: {}}).mode;
};

console.log(mode([3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3, 2, 3, 8, 4, 6]));
//=> 3

To see how it works, if you drop off the .mode at the very end of the return, you will get this object instead:

{
    "mode": 3,
    "greatestFreq": 4,
    "numMapping": {"1": 2, "2": 2, "3": 4, "4": 2,
                   "5": 3, "6": 2, "7": 1, "8": 2, "9": 3}
}

This object is built up a piece at a time by the reduce call running over each element of the array, updating the numMapping each time and the greatestFreq and mode when appropriate.

Answer 3

This is not bad at all. A few things can be simplified a bit.

---

When checking if arr[i] exists in numMapping:

if(numMapping[arr[i]] === undefined){

A simpler and more natural way is this:

if (!(arr[i] in numMapping)) {

---

As a tiny optimization, instead of this:

if (!(arr[i] in numMapping)) {
    numMapping[arr[i]] = 0;
}
numMapping[arr[i]] += 1;

This is somewhat better:

if (!(arr[i] in numMapping)) {
    numMapping[arr[i]] = 1;
} else {
    numMapping[arr[i]] += 1;
}

Because this way you save one extra lookup of arr[...] and also numMapping[...], by directly assigning 1 instead of 0 and then incrementing later.

---

At a few places the indentation was off, and I would recommend a bit more generous spaces in if(...){ and for(...){ statements, use if (...) { and for (...) { instead (like in my examples above).

Answer 4

You might appreciate taking advantage of some ES6 features. You're using an object literal like a map, so why not use a real Map? Speaking of ES6, since you're a beginner, you would benefit from taking the time to learn it now and save yourself the headache later. As others have noted, you can accomplish finding the mode in a single for loop.

const mode = (array) => {
  const map = new Map();
  let maxFreq = 0;
  let mode;

  for(const item of array) {
    let freq = map.has(item) ? map.get(item) : 0;
    freq++;

    if(freq > maxFreq) {
      maxFreq = freq;
      mode = item;
    }
    
    map.set(item, freq);
  }

  return mode;
};

const testArray = [1, 1, 2, 3, 5, 8, 13];
console.log(`Mode of [${testArray}] is ${mode(testArray)}.`);