CODEWITHSUNDEEP
javascriptobjectcounter
ScaredSquirrel
ScaredSquirrel

Reputation: 65

JavaScript Object as counter not incrementing

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')

Upvotes: 0

Views: 130

Answers (6)

Niloct
Niloct

Reputation: 10015

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');

Upvotes: -1

Berthur
Berthur

Reputation: 4495

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.

Upvotes: 1

Osama
Osama

Reputation: 3040

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);

Upvotes: 0

Ele
Ele

Reputation: 33726

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; }

Upvotes: 1

Jonas Wilms
Jonas Wilms

Reputation: 138277

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.

Upvotes: 1

Mark
Mark

Reputation: 92440

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))

Upvotes: 2

Related Questions