Find number repeated odd times in array I completed the following exercise:


  You are given an array of repeating numbers. All numbers repeat in an even way, except for one. Find that odd occurring number.


function findOdd(arr){
  obj = {};

  for (var el in arr) {
    if (!obj[arr[el]]) {
      obj[arr[el]] = 1;
    } else {
      obj[arr[el]]++;
    }
  }

  for (var key in obj) {
    if(obj[key] % 2 === 1){
      obj = {'odd': key}
    }
  }
  return parseInt(obj['odd']);
}

findOdd([1,1,2,4,4,4]);


I am trying to figure out if there are better ways to implement this. A few things come to mind:


I believe I can nest the for loops together. I'm not sure if this saves any space or is preferable styling wise.
I re-assigned the created object in the function so it just holds the result. I figured it's a bit more readable. Should I have just returned it in the second for loop?
The code seems 'dirty' to me. What might be another possible solution?
Yes dirty, buggy, and bad.

There are several problems with your code. You have undeclared variables, do not protect against iterating over unintended enumerable array properties, redundant code, inappropriate object types, and unneeded iteration.

Undeclared variable.

You use the variable obj without declaring it. It is thus created in global scope and available to any function. This can result in difficult to spot bugs and unexpected behaviour.

Always on the first use of a variable declare it with one of JavaScript's variable types var, let, or const. In this case I will not say which type to use as the variable obj is badly used and inappropriate for the situation.

Use strict

To save you from ever make this mistake again you should add to the first line of any javascript file or script tag the line 

"use strict";


this will enforce some stricter parsing standards and help you avoid making mistakes such as "forgetting to declare". It also has the benefit of running your code quicker.

Which way to iterate.

There are many ways in javascript to iterate over a set of items in arrays, objects, iterable objects, and array like objects. Using built in statements for, for...in, for...of, while, do...while, and more

The one you use is about the worst for the situation. The problem with for...in is that it will iterate over all the arr/object enumerable properties, including properties that are inherited via the object's prototype.

This means you can get almost anything for each iteration, and thus need to check every item to see if it is what you intend to be handling, and if it belongs to the object or has it be inherited from another. This is just a pain, so much so I never use it.

For your code you should have used 

for(const el of arr){ 


Which does what you intended without the risk of getting unknown properties.

Dont burn cycles

Good code is efficient code, with every instruction the computer executes in your code, you burn power, sucked from batteries, the grid, You chew the clients time and you extract a little more from the environment we share as a comunity. 

Yes your code is trivial in the scheme of things, immeasurable, but combined millions of programmes, or with fortune on your side an app you write  may end up on billions of devices, and particular parts of your code may execute trillions of times a minute. Then it matters.

Always (I digress, no rant... :P) code for efficiency, even the little bits combined matter.

Your code could have exited early as the problem states that there is only one set of odd numbers in the array. So as soon as you find the odd set, the job is over and you should exit. On average you will do a quarter as much processing overall. On average you could do up to a quarter less processing overall.

Use appropriate Objects.

You are using obj as a map, for each unique key (number) you create an entry and use the associated value to count the number of keys. Though technically not wrong javascript provides a object Map that is better suited (see rewrite)

The rewrite

If the array of numbers was known to be sorted or numbers were grouped then a different solution could be used that is more efficient, But as there is no mention of the array being sorted or numbers grouped apart from the sample data the best is to play safe and assume unsorted data.

Some may opt to sort the array to take advantage of the better solution, but the sort is expensive and would offset any benefits the more efficient method would give.

UPDATE I miss read the question and thought that counts of one were to be excluded. I have changed the code via commenting out the incorrect behaviour.

function findOdd( arr ) {
    const counts = new Map(); 
    // find each unique number and count the entries
    for (const num of arr) {
        const item = counts.get(num); // get entry with key = num
        if (item !== undefined) {     // does it exist
            counts.set(num, item + 1);          // yes count one more
        } else {
            counts.set(num, 1);       // no so add and set count at 1
        }
    }
    // all items in the array have been checked so find the first with odd count
    for (const [num, count] of counts) {
        if (/*count &gt; 2 &amp;&amp;*/ count % 2) { // See update in answer re comment
            return num; // we have found what we are looking for
        }
    }
    // the problem suggests that there is always an odd set so this
    // line will never be reached.
}


Sorted version

Just for the exercise I have included the more efficient code that assumes the array of numbers is sorted or that numbers are in groups.

function findOdd (arr) {
    var i = arr.length - 1;
    var count = 1;
    var prev = arr[i];
    while (i--) {
        const current = arr[i];
        if (current !== prev) {
            if (count % 2) { return prev }  // Update remove test for count &gt; 2
            count = 1;
        } else { count += 1 }
        prev = current;
    }
}


Which on average executes in 1/3rd the time of the other method