Function that returns the number of items that appear more than once

Question

This is the question:

Write a method that takes in a string and returns the number of letters that appear more than once in the string. You may assume the string contains only lowercase letters. Count the number of letters that repeat, not the number of times they repeat in the string.

How can I make this better?

function count(str){
    var nArr = [];
    var result = [];
    var obj = {};
    var arr = str.split("");
    // count all occurances in arr
    for(var i = 0; i < arr.length; i++){
        var letter = arr[i];
        obj[letter] === undefined ? obj[letter] = 1 : obj[letter] += 1;

    }
    for(var prop in obj){
        if(obj[prop] !== 1){
            nArr.push(prop);
        }
    }
    return nArr.length;

}

count("adsaaaffda");

Answer 1

The name count is too vague for such a specific task. I suggest countReappearingChars instead.

Avoid big blocks of variable definitions. Code is easier to read and maintain when variables are declared and initialized near where they are used. For example, with a block like this, it's harder to notice that result is never used.

nArr does not need to be an array. You can just keep a count.

obj is cryptically named. I suggest calling it charCount.

var arr = str.split("") is unnecessary; you can iterate over str directly.

"occurances" is misspelled in the comment.

---

This line is hard to read:

obj[letter] === undefined ? obj[letter] = 1 : obj[letter] += 1;

It would be more clearly be written using if-else, and therefore should be:

if (obj[letter] === undefined) {
    obj[letter] = 1;
} else {
    obj[letter] += 1;
}

I would consider your ternary operator to be abusive, since you are not using it as as an expression. In contrast, this expression could be considered appropriate, since the right-hand side of the assignment is an expression:

obj[letter] = (obj[letter] === undefined) ? 1 : obj[letter] + 1;

But that still looks cumbersome. If you want to get clever with it, you could use || 0, as shown below.

---

function countReappearingChars(str) {
    // Count all occurrences in str
    var charCount = {};
    for (var i = 0; i < str.length; i++) {
        charCount[str[i]] = 1 + (charCount[str[i]] || 0);
    }

    // Loop over and filter charCount
    var result = 0;
    for (var char in charCount) {
        if (charCount[char] > 1) result++;
    }
    return result;
}

Answer 2

The @200_success's answer is essentially a code review, strictly speaking, and I agree with its recommendations.

In the other hand it keeps a 2-steps strategy like in the OP, so I suggest a slightly different approach, using only 1 loop:

function countRepeatedChars(str){
  var repChars = '';
  for (var i = 0, n = str.length; i <n; i++) {
    var letter = str[i];
    if (!repChars.includes(letter) && str.includes(letter, i + 1)) {
      repChars += letter;
    }
  }
  return repChars.length;
}

countRepeatedChars("adsaaaffda");

If not already registered as repeated, each character is checked to appear after its own position in the original string, then registered if yes.

It results in a reduced code, and might be a bit faster.