Why does the last character keeps on printing repeatedly?

Question

I have written the following to animate the text in a div, but I cannot find how does the last character gets printed repeatedly.

var textClass = $(".first-text");
var text = textClass.text();
textClass.text("");
for (var i in text) {
  $(textClass).animate({
    opacity: 0.25
  }, 200, function() {
    $(textClass).append(text.charAt(i));
  });
}

p:not(:first-child) {
  display: none;
}

p {
  margin: 0 auto;
  font-size: 24px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="animate-text">
  <p class="first-text">HTML</p><br>
</div>

If I try to the alert the value of i or text.charAt(i), I always get the desired output, but when I try to append the same in a div, I always get the same last letter that is printed repeatedly. I cannot find where I am mistaken. I cannot the find the bug in my logic.

If anyone could enlighten me on my mistake in the above code, I would be glad to hear it.

Here is the link to my fiddle where I tried this code.

Thanks in advance.

Switching `var i in text` to `const i in text` solves the problem. Then it becomes a scoped variable and not global as it dose with `var` — Endless, Jul 30 '18 at 11:12

Blue · Accepted Answer · 2018-08-03T14:00:31.087

You've stumbled into a bit of learning when it comes to closures. When i loops through, and eventually gets run inside the function, it's only looking at the last character, because that's what i was overwritten to before the first animate() actually fires.

You can counteract this by manually creating a closure yourself, wrapping it in a function and passing it in, to preserve the variable at the time of the loop.

For more information on closures, check out: What is a 'Closure'?

var textClass = $(".first-text");
var text = textClass.text();
textClass.text("");
for (var i in text) {
  (function (char) {
    $(textClass).animate({
      opacity: 0.25
    }, 200, function() {
      $(textClass).append(text.charAt(char));
    });
  })(i)
}

p:not(:first-child) {
  display: none;
}

p {
  margin: 0 auto;
  font-size: 24px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="animate-text">
  <p class="first-text">HTML</p><br>
</div>

Alternatively, you can use new let or const syntax, which defines i for the scope of the block (Which essentially creates a closure around your if block.)

var textClass = $(".first-text");
var text = textClass.text();
textClass.text("");
for (const i in text) {
  $(textClass).animate({
    opacity: 0.25
  }, 200, function() {
    $(textClass).append(text.charAt(i));
  });
}

p:not(:first-child) {
  display: none;
}

p {
  margin: 0 auto;
  font-size: 24px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="animate-text">
  <p class="first-text">HTML</p><br>
</div>

thanks a lot for your answer, really helped me understand the logic behind my bug. — Code_Ninja, Jul 30 '18 at 11:26
But why not const for the other two constants? They are not changing either — mplungjan, Jul 30 '18 at 11:27
No problem @Code_Ninja. Javascript can get really nasty with closures, and the `let/const` syntax really helps clarify the scope of variables. I pretty much use them everywhere, and stray away from using `var` at all. — Blue, Jul 30 '18 at 11:28
@mplungjan He can certainly, and I would recommend doing that, but this was not the scope of the issue he encountered, so I tried to avoid changing code unnecessarily. — Blue, Jul 30 '18 at 11:28

Mamun · Answer 2 · 2018-07-30T11:25:26.993

You can either create a closure or use let or const to declare the variable i inside for loop which will preserve the current value of i in each iteration:

var textClass = $(".first-text");
var text = textClass.text();
textClass.text("");
for (const i in text) {
  $(textClass).animate({
    opacity: 0.25
  }, 200, function() {
    $(textClass).append(text.charAt(i));
  });
}

p:not(:first-child) {
  display: none;
}

p {
  margin: 0 auto;
  font-size: 24px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="animate-text">
  <p class="first-text">HTML</p><br>
</div>

i would use const when ever possible to indicate that the variable are not being changed anywhere else in the code — Endless, Jul 30 '18 at 11:18

score 1 · Answer 3 · answered Jul 30 '18 at 11:17

1

Using the let and const instead of var you get a better scoping and do not need to create a closure. Also no need to keep doing $(textClass) - you can cache the object

const $textClass = $(".first-text");
const text = $textClass.text();
$textClass.text("");
for (let i in text) {
  $textClass.animate({
    opacity: 0.25
  }, 200, function() {
     $textClass.append(text.charAt(i));
  });
}

p:not(:first-child) {
  display: none;
}

p {
  margin: 0 auto;
  font-size: 24px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="animate-text">
  <p class="first-text">HTML</p><br>
</div>

answered Jul 30 '18 at 11:17

mplungjan

169,008
28
173
236

1

thanks @mplungjan for the fix, it worked and I wonder whats the difference if I use `var i` or `let i` or `const i`. – Code_Ninja Jul 30 '18 at 11:20
Even though OP is wrapping his `textClass` in `$()` unnecessarily, it really doesn't cause any overhead. jQuery will see it as a jQuery object, and simply return it. – Blue Jul 30 '18 at 11:23
It is still inelegant in my mind - there will be some cases where it does not and then you add overhead – mplungjan Jul 30 '18 at 11:25

score 1 · Answer 4 · answered Jul 30 '18 at 14:03

It seems to have a variable declaration in your script.

var textClass = $(".first-text");
var text = textClass.text();
textClass.text("");
for (const i in text){
  $(textClass).animate({
    opacity: 0.25
  }, 200, function(){
    $(textClass).append(text.charAt(i));
  });
}

Please review the following JSFiddle link. http://jsfiddle.net/tp3juw54/19/

Why does the last character keeps on printing repeatedly?

4 Answers4