JavaScript Object as counter not incrementing

Question

I'm trying to use objects as a counter in JavaScript, to find the most repeated letter in a word, but somehow the function is not working as I intend to. For example, when I call findMaxRepeatCountInWord('expected'), the letterCount object ends up with a value of { e: 1, x: 1, p: 1, c: 1, t: 1, d: 1 }. What's wrong with my code? Shouldn't e has a value of 3?

My code:

function findMaxRepeatCountInWord(word){
  var letterCount = {};
  word.split('').map(function(v){
    if(letterCount[v] != true){ letterCount[v] = 1 } 
    else(letterCount[v] ++)
  });
  return Object.values(letterCount).sort((a,b) => b-a)[0]
}

findMaxRepeatCountInWord('expected')

It's not....I tried changing the condition within the else{} statement to 'console.log(v)' and it worked fine - it would print out 'e' twice. So the function definitely flew into the else{} statement, I just don't understand why the action within is not implemented. — rhea.rao, Oct 27 '18 at 22:30

Mark · Answer 1 · 2018-10-27T22:32:35.263

2

This will not work the way you are hoping:

 if(letterCount[v] != true)

Try testing with !hasOwnProperty (and using forEach instead of map) instead:

function findMaxRepeatCountInWord(word){
    var letterCount = {};
    word.split('').forEach(function(v){
      if(!letterCount.hasOwnProperty(v)){ letterCount[v] = 1 } 
      else letterCount[v] ++
    });
    console.log("letter counts", letterCount)
    return Object.values(letterCount).sort((a,b) => b-a)[0]
  }
  
  console.log(findMaxRepeatCountInWord('expected'))

You can also save the cost of a sort with:

return Math.max(...Object.values(letterCount))

edited Oct 27 '18 at 22:32

answered Oct 27 '18 at 22:28

Mark

90,562
7
108
148

1

Not always ... only for 1 – Jonas Wilms Oct 27 '18 at 22:31
Thanks for the suggestion, I don't think the `Math.max` function works for an array of number though does it? – rhea.rao Oct 27 '18 at 22:44
@rhea.rao it does – Jonas Wilms Oct 27 '18 at 22:46
@rhea.rao `Object.values(letterCount)` returns an array or numbers which is spread to `Math.max`. – Mark Oct 27 '18 at 22:47
`Math.max(1,2,3)` would return `3`, but `Math.max([1,2,3])` would return `NaN` – rhea.rao Oct 27 '18 at 22:55
@rhea.rao what about `Math.max(...[1,2,3])` (note the dots aka [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax)). – Mark Oct 27 '18 at 22:56

Ele · Answer 2 · 2018-10-27T22:39:31.150

You need to check for the property as follow: if (v in letterCount) this is the most reliable way to check if a key is a property of an object.

This approach returns the built object

function findMaxRepeatCountInWord(word) {
  var letterCount = {};
  word.split('').map(function(v) {
    letterCount[v] = (letterCount[v] || 0) + 1;
  });
  
  return letterCount;
}

console.log(findMaxRepeatCountInWord('expected'))

.as-console-wrapper { max-height: 100% !important; top: 0; }

Aside note, using the function Array.prototype.map is not the best way to solve this problem because you're creating an unnecessary array.

Alternative approach using the function Array.prototype.reduce to group the letters

function findMaxRepeatCountInWord(word) {
  return word.split('').reduce((a, c) => {
    a[c] = (a[c] || 0) + 1;
    return a;
  }, Object.create(null));
}

console.log(findMaxRepeatCountInWord('expected'))

.as-console-wrapper { max-height: 100% !important; top: 0; }

score 1 · Accepted Answer · answered Oct 27 '18 at 22:33

Thats another version of JS comparison fun:

 console.log(
   0 != true, // true
   1 != true, // false
   2 != true // true
   3 != true // true
);

If you compare a number and a boolean, the boolean is turned into a number (0 -> false, 1 -> true) then equality comparison is done, and therefore only 1 equals true. Therefore if it reaches 2, it will enter the first branch again and reset the counter to 1.

score 1 · Answer 4 · answered Oct 27 '18 at 22:49

Your code is correct, apart from the if-condition: if(letterCount[v] != true).

The solution: You can fix the problem easily by just changing the condition to the following: if(letterCount[v] == undefined)

The problem: The condition works as you intended when:

letterCount[v] is undefined
letterCount[v] is 1

But when letterCount[v] becomes greater than 1, the condition is true again, because 2 != true is true. For this reason your code will reset the values to 1 every time it has been encountered an odd number of times.

To illustrate, if we run your code on a word with only two e's, we obtain:

findMaxRepeatCountInWord('expectd')
> 2

but if there is a third occurrence of the letter, as in your example, the number will be reset to 1.

Performance Nevertheless, this approach is not computationally very efficient. For some discussion regarding efficient solutions of an analogous problem, you may want to give e.g. this question a look.

score 0 · Answer 5 · answered Oct 27 '18 at 22:42

function findMaxRepeatCountInWord(word){
  var letterCount = {};
  word.split('').map(function(v){
    if(letterCount[v]==undefined){ letterCount[v]= 1 ;
} 
    else{letterCount[v]=letterCount[v]+1}
  });
  return Object.values(letterCount).sort((a,b) => b-a)[0]
}

var x=findMaxRepeatCountInWord('expected');
console.log(x);

Niloct · Answer 6 · 2018-10-28T00:44:27.473

-1

The if should be:

if(!letterCount[v]){ letterCount[v] = 1 } 
else {letterCount[v]++}

To keep the code minimally modified. Other answers polish it more.

Applied to the OP code:

function findMaxRepeatCountInWord(word){
  var letterCount = {};
  word.split('').map(function(v){
    if(!letterCount[v]){ letterCount[v] = 1 } 
    else(letterCount[v] ++)
  });
  console.log(letterCount)
  return Object.values(letterCount).sort((a,b) => b-a)[0]
}

findMaxRepeatCountInWord('super expected');

edited Oct 28 '18 at 00:44

answered Oct 27 '18 at 22:27

Niloct

9,491
3
44
57

using `{}` for `if/else` is not mandatory although it is best practice – charlietfl Oct 27 '18 at 22:32
Right, but the original code has parenthesis in place of brackets, which is important to change too. – Niloct Oct 27 '18 at 22:33
Not really...is ugly but valid – charlietfl Oct 27 '18 at 22:34
Hm you're right. Works like a expression grouping ? The main issue was the `if` indeed. – Niloct Oct 27 '18 at 22:37
sorry this is not correct, 0 == false, 1 == true, 2 == false, so when we count to 2, `letterCount[v]` would be assigned the value of 1 again, see the answers above – rhea.rao Oct 27 '18 at 23:25

JavaScript Object as counter not incrementing

6 Answers6