0

I have an script that prints each letter of an string and how many times it is printed, but it is not working properly, it overrides the last letter. For example: when you type in the textarea "assist" it should print the following: a: 1 s: 3 i: 1 t: 1 But it only prints: t: 1 How to print properly the string?

function ex44() {
  let string = document.getElementById("sarc4").value
  string = string.toLowerCase();
  const count = {}
  string.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1
      return
    }
    count[ch]++
  })
  for (const key in count) {
    document.getElementById("result4").innerHTML = `${key} : ${count[key]}`
  }
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5"></textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>
Andy
  • 61,948
  • 13
  • 68
  • 95
Mihai012
  • 31
  • 3
  • 2
    unsurprising, since you are *overwriting* the `.innerHTML` of `#result4` for each `key` in `count`. – Alnitak Sep 20 '21 at 14:39
  • It's in your `for(const key incount)` loop. – Dan Zuzevich Sep 20 '21 at 14:39
  • 1
    you need to accumulate those entries into a string using `+=`, and then just write that _once_ into `#result4` _outside the loop_. – Alnitak Sep 20 '21 at 14:40
  • Thanks guys, sorry for disturbing, I am new to Javascript – Mihai012 Sep 20 '21 at 14:43
  • @Andy no, no, no. Never use `+=` on `.innerHTML`. It requires the browser to convert the HTML back into a string, and then you modify the string, and then the browser has to convert it back into DOM, _for every iteration_. – Alnitak Sep 20 '21 at 14:52

4 Answers4

1
document.getElementById("result4").innerHTML = `${key} : ${count[key]}`

Will override the innerHTML.

Consider 'adding' the content using appendChild or insertAdjacentHTML:
Is it possible to append to innerHTML without destroying descendants' event listeners?

const res = document.getElementById("result4");
for (const key in count) {
    res.insertAdjacentHTML('beforeend', `<p>${key} : ${count[key]}</p>`);
}

function ex44() {
  let string = document.getElementById("sarc4").value
  string = string.toLowerCase();
  const count = {}
  string.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1
      return
    }
    count[ch]++
  })
  const res = document.getElementById("result4");
  for (const key in count) {
    res.insertAdjacentHTML('beforeend', `<p>${key} : ${count[key]}</p>`);
  }
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5"></textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>

Note: I've wrapped the ${key} : ${count[key]} inside a <p></p> tag so we see each letter on a separate row


Another option is to create an output string by using map and join like so:

const output = Object.keys(count).map((key) => `<p>${key} : ${count[key]}</p>`);
document.getElementById("result4").innerHTML = output.join('');

function ex44() {
  const input = document.getElementById("sarc4").value.toLowerCase();
  const count = {}
  
  input.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1;
    } else {
      count[ch]++
    }
  });
   
  const output = Object.keys(count).map((key) => `<p>${key} : ${count[key]}</p>`);
  document.getElementById("result4").innerHTML = output.join('');
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5">test</textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>
0stone0
  • 34,288
  • 4
  • 39
  • 64
  • 1
    No, no, no. Never use `+=` on `.innerHTML`. It requires the browser to convert the HTML back into a string, and then you modify the string, and then the browser has to convert it back into DOM, destroying and recreating the DOM elements, _for every iteration_. – Alnitak Sep 20 '21 at 14:57
  • You're right @Alnitak, trying to be too fast. I've improved my answer. – 0stone0 Sep 20 '21 at 15:00
0

Move the document.getElementById("result4").innerHTML outside the loop.

You are rewritting the innerHTML of the element with id result4 each time the loop is executing. You can update the innerHTML by appending the new striing with existing by using below to get the issue fixed.

document.getElementById("result4").innerHTML += `${key} : ${count[key]}`

The above implementation cannot be considered as a good implementation even though it's fixes your problem. Accessing DOM elements directly from script is always costly. If it's inside a loop a loop it's definitely more costly.

Instead of accessing the DOM element multiple time from a loop, you could create the template string inside the loop and assign the innerHTML only once as below. This will be better in terms of performance.

function ex44() {
  let string = document.getElementById("sarc4").value
  string = string.toLowerCase();
  let template = '';
  const count = {}
  string.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1
      return
    }
    count[ch]++
  })
  for (const key in count) {
    template += `${key} : ${count[key]}`
  }
  document.getElementById("result4").innerHTML = template;
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5"></textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>
Nitheesh
  • 19,238
  • 3
  • 22
  • 49
  • see other comments about why you shouldn't use `+=` with `.innerHTML`. Please, just remove that from your answer entirely. – Alnitak Sep 20 '21 at 15:05
  • I too don't suggest to use += with innerHTML. Please see updated answer. – Nitheesh Sep 20 '21 at 15:24
  • 1
    _someone_ will read your first line of code, and think it's a good idea, without reading the following text about the consequences. It isn't even just about performance. Using `+=` destroys the original child elements, _including any event handlers registered on them_. – Alnitak Sep 20 '21 at 15:31
0

You should use Object.keys(count) to foreach. And currently, you are overwriting the result's HTML on loop. So I use a newHtml variable to save it and set it into innerHTML outside loop.

function ex44() {
  let string = document.getElementById("sarc4").value
  string = string.toLowerCase();
  const count = {}
  string.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1
      return
    }
    count[ch]++
  })
  
  var newHtml = '';
  for (const key of Object.keys(count)) {
    newHtml += `${key} : ${count[key]} <br/>`
  }
document.getElementById("result4").innerHTML = newHtml;
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5"></textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>
Tuan Dao
  • 2,647
  • 1
  • 10
  • 20
  • there's absolutely nothing wrong with `for (const key in count)`. No sane person adds enumerable properties to `Object.prototype` any more. – Alnitak Sep 20 '21 at 15:03
  • 1
    alternatively use `for (const [ch, n] of Object.entries(count))` and get the key _and_ value together. – Alnitak Sep 20 '21 at 15:29
  • @Alnitak thanks for your suggestion. It's a good idea. – Tuan Dao Sep 20 '21 at 15:33
0

Cache your elements, and then use textContent to update the element text using string concatenation.

const sarc4 = document.getElementById('sarc4');
const result4 = document.getElementById('result4');

function ex44() {
  string = sarc4.value.toLowerCase();
  const count = {};
  string.split("").forEach(ch => {
    if (!count[ch]) {
      count[ch] = 1;
      return;
    }
    count[ch]++
  });
  let result = '';
  for (const key in count) {
    result += `${key}:${count[key]} `;
  }
  result4.textContent = result;
}
<label for="sarcina4"> Enter text</label>
<br>
<textarea name="sarcina4" id="sarc4" cols="60" rows="5"></textarea>
<br>
<button onclick="ex44()">Afisare</button>
<p id="result4"></p>
Andy
  • 61,948
  • 13
  • 68
  • 95
  • I wouldn't use `+=` on `.textContent` either - just accumulate into a string and update the DOM _once_. – Alnitak Sep 20 '21 at 15:05