Why is this text effect not working?

Question

I am trying to modify the script from here to work on more then one span.

I have tried this, but it seems to overwrite both spans with the same text.

<html>
<head>
<script>
var got;
var chars;

function change(decSpan,encSpan)
{
   var randstring = "";
   var rslength = chars.length - got.length;

   var decrypted = document.getElementById(decSpan);
   var encrypted = document.getElementById(encSpan);

   for(var x=0;x<rslength;x++)
   {
       i = Math.floor(Math.random() * chars.length);
       randstring += chars.charAt(i);
   }

   if(randstring.charAt(0) == chars.charAt(got.length))
   {
      got += randstring.charAt(0);
      decrypted.innerHTML = got;
   }
   else
   {
      encrypted.innerHTML = randstring;
   }

   if(chars.length > got.length)
   {
  setTimeout("change('"+decSpan+"','"+encSpan+"')", 10);
   }
   else
   {
      encrypted.innerHTML = "";
   }
}
function startdecrypt()
{
   var decodeSpans = ["decoded","decoded2"];
   var encodeSpans = ["encoded","encoded2"];
   for(var z in decodeSpans) 
   {
decSpan = decodeSpans[z];
encSpan = encodeSpans[z];
    var decrypted = document.getElementById(decSpan);
    var encrypted = document.getElementById(encSpan);

    chars = decrypted.innerHTML;
    decrypted.innerHTML = "";
    got = "";
    setTimeout("change('"+decSpan+"','"+encSpan+"')", 10);
   }
}
</script>
</head>
<body bgcolor="#FFFFFF" text="#000000">
<input type="button" value="go" onClick="javascript:startdecrypt()"><br>
<span id="decoded">Test1</span><span id="encoded"></span><br>
<span id="decoded2">Test2</span><span id="encoded2"></span>
</body>
</html>

When the button is clicked, the texts change randomly but still use all characters from the original texts. If this is what you want, it works for me. What do you mean by "no working"? — NawaMan, Jul 18 '10 at 16:27

score 2 · Answer 1 · answered Jul 18 '10 at 16:29

The problem is that that script uses global javascript variables: chars and got are set in startdecrypt and used later in change function. Thus, the next iteration of loop overrides previously set values.

The best solution is probably to include them in js call of change function, like you do with ids.

Also, make sure to declare all js variables local to avoid such side-effects: var got = "".

score 0 · Answer 2 · edited May 23 '17 at 11:48

I'll admit not to have gone through the whole of your code, but it seems you made an unintended Javascript closure, like in this example:

Javascript closures - variable scope question

Or, as Nikita put it, global variables, which, when refered to in the change() function make a closure. Note that the var inside the for does not change anything. Javascript does not have block scope.

Note that using var inside the loop or declaring var x at the top of the function has the same effect. You might want to consider using let instead:

Let statemennt from Mozilla Developper

Closures are hard to understand, especially unintended ones. Ender's answer on the link above has helpful links about how they work.

score 0 · Answer 3 · answered Jul 18 '10 at 17:22

If I understand correctly, this should do what you're looking for:

var decodeSpans = ["decoded", "decoded2"];
var encodeSpans = ["encoded","encoded2"];

function encoder(decSpan, encSpan){
    this.encSpan = encSpan;
    this.decSpan = decSpan;
    var got = "";
    var chars = decSpan.innerHTML;
    decSpan.innerHTML = "";
    return (function(){
        var randstring = "";
        var rslength = chars.length - got.length;

        for(var x=0;x<rslength;x++){
            i = Math.floor(Math.random() * chars.length);
            randstring += chars.charAt(i);
        }

        if(randstring.charAt(0) == chars.charAt(got.length)){
            got += randstring.charAt(0);
            decSpan.innerHTML = got;
        }else{
            encSpan.innerHTML = randstring;
        }

        if(chars.length > got.length){
            decSpan.innerHTML = chars;
            encSpan.innerHTML = "";
        }else{
            encSpan.innerHTML = "";
        }
    })();
}

function startdecrypt(){
    for(var z = 0; z < decodeSpans.length; z++){
        decSpan = decodeSpans[z];
        encSpan = encodeSpans[z];
        var decrypted = document.getElementById(decSpan);
        var encrypted = document.getElementById(encSpan);
        encoder(decrypted, encrypted);
    }
}

I only read up properly on closures like 2 nights ago, so still trying to put it all into context, so some (constructive) feedback on this solution would be welcome from those who know about these things :)

Why is this text effect not working?

3 Answers3