2

I want to count all characters in a string and return it to an object. I have tried but I’m unable to get the correct answer.

This is my code:

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  a.forEach(function(s){
    var count=0;
    for(var j=0;j<a.length;j++){
      if(s==a[j]){
        count+=1;
      }
      obj[a[j]]=count;
    }
  });
  return obj;
}
console.log(countAllCharacters('banana'));

Output:

{ b: 0, a: 3, n: 2 } 

Which obviously is wrong.

Can anyone help me with that? Where I am going wrong?

Sebastian Simon
  • 16,564
  • 7
  • 51
  • 69

9 Answers9

5

The minimum necessary change is that obj[a[j]]=count; should be obj[s]=count;, because you have that line running on every iteration of the inner loop regardless of whether j refers to the letter you're currently tallying.

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  a.forEach(function(s){
    var count=0;
    for(var j=0;j<a.length;j++){
      if(s==a[j]){
        count+=1;
      }
      obj[s]=count;
    }
  });
  return obj;
}
console.log(countAllCharacters('banana'));

However, you don't need a nested loop. Your outer .forEach() can update the count for the current letter directly:

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  a.forEach(function(s){
    obj[s] = (obj[s] || 0) + 1;
  });
  return obj;
}
console.log(countAllCharacters('banana'));

This can be made shorter with .reduce():

function countAllCharacters(str) {
  return str.split("").reduce(function(obj, s){
    obj[s] = (obj[s] || 0) + 1;
    return obj;
  }, {});
}
console.log(countAllCharacters('banana'));

Note that (obj[s] || 0) means to use obj[s]'s value if it is truthy, otherwise use 0. So the first time you encounter a particular letter obj[s] will be undefined, which is falsey, so then 0 will be used. The next time you encounter that letter obj[s] will be 1, which is truthy.

nnnnnn
  • 143,356
  • 28
  • 190
  • 232
4

I'd use a reduce operation, like so

const str = "banana"
const charCounts = Array.from(str).reduce((counts, char) => {
  counts[char] = (counts[char] || 0) + 1
  return counts
}, Object.create(null))

console.info(charCounts)
Phil
  • 141,914
  • 21
  • 225
  • 223
3

Actually you can count by better performance, you loop more than you needed!

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  for(var j=0;j<a.length;j++){
    if(typeof obj[a[j]] !== 'undefined'){
      obj[a[j]]+=1;
    } else {
      obj[a[j]]=1;
     }
  }
  return obj;
}
console.log(countAllCharacters('banana'));
Mohammad Hamedani
  • 3,269
  • 3
  • 9
  • 22
1

I think you need not to use a forEach and a for loop together when you can do it with only foreach. Here is the code.

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  a.forEach(function(s) {
    if (obj[s]) {
      obj[s] = obj[s] + 1;
    } else {
      obj[s] = 1;
    }
  });
  return obj;
}

console.log(countAllCharacters('banana'));

Hope it helps :)

Manish
  • 4,458
  • 3
  • 30
  • 40
1

The problem here is that you assign the obj[a[j]] = count; when the counting is not yet complete. You should do change your function(s) to this:

  function(s){
    var count=0;
    for(var j=0;j<a.length;j++){
      if(s==a[j]){
        count+=1;
      }
    }
    obj[s]=count;
  }

Another comment: you code is very inefficient, which is O(n^2). You can simplify it much further to get an O(n) algorithm with this:

  function(s){
    if (obj[s] == undefined) {
       obj[s] = 1;
    } else {
       obj[s] = obj[s] + 1;
    } 
  }
Dat Nguyen
  • 1,571
  • 20
  • 24
1

Already so many friends submitted their opinions and cool solutions. Here is my solution with the simplest code:

const src = 'banana';

const count = str => 
  [...str].reduce((acc, val) => (acc[val] ? (acc[val]++) : (acc[val]=1), acc),{});
  
console.log(count(src));
.as-console-wrapper {min-height: 100%}

Hope you enjoy my code. Thanks.

Yevgen Gorbunkov
  • 14,071
  • 3
  • 15
  • 36
Md. Jamal Uddin
  • 638
  • 7
  • 16
0

I realize this isn't the prettiest, but I'm hoping it shows how you can use console to help debug loops.

function countAllCharacters(str) {
  var a = str.split("");
  var obj = {};
  a.forEach(function(s){
  console.log(s);
    var count=0;
    for(var j in a){

      // commas can come in really handy and help avoid huge debug blocks.
      console.log('_',j,s,a[j]); 

      if(s==a[j]){
        console.log('count++');
        count++;
      }
      obj[s] = count;
    }
  });
  return obj;
}

console.log(countAllCharacters('banana'));
admcfajn
  • 1,895
  • 3
  • 21
  • 29
0

you have to bring obj[a[j]]=count inside your if statement.

this should return correct results

function countAllCharacters(str) {
  var a = str.split("")
  var obj = {}
  a.forEach(function(s){
    var count=0
    for(var j=0;j<a.length;j++){
      if(s===a[j]){
        count+=1
        obj[a[j]]=count
      }
    }
  })
  return obj
}
console.log(countAllCharacters('banana'))

the increment in obj must also satisfy the if statement.

phtn458
  • 465
  • 5
  • 11
0
function countAllCharacters(str) {
   let newArr = str.split('');  // splits the string into a new array
   let obj = {};  // initiliza an empty object

   newArr.forEach(char => (obj[char]) ? obj[char] += 1 : obj[char] = 1);
   return obj; 
}