Join the Stack Overflow Community
Stack Overflow is a community of 6.6 million programmers, just like you, helping each other.
Join them; it only takes a minute:
Sign up

I'm trying to make a click handler that calls a function; and that function gets a string and basically slices the last character and adds it to the front, and each time you click again it should add the last letter to the front.

It seem so easy at first that I thought I could just do it using array methods.

 function scrollString() {

    var defaultString = "Learning to Code Javascript Rocks!";
    var clickCount = 0;

    if (clickCount === 0) {
        var stringArray = defaultString.split("");
        var lastChar = stringArray.pop();
        stringArray.unshift(lastChar);
        var newString = stringArray.join('');
        clickCount++;

    } else {
        var newArray = newString.split("");
        var newLastChar = newArray.pop();
        newArray.unshift(newLastChar);
        var newerString = newArray.join("");
        clickCount++;
    }

    document.getElementById('Result').innerHTML = (clickCount === 1) ? newString : newerString;

}

$('#button').on('click', scrollString);

Right now it only works the first time I click, and developer tools says newArray is undefined; also the clickCount stops incrementing. I do not know if it's an issue of scope, or should I take a whole different approach to the problem?

share|improve this question

Every time you click you are actually reseting the string. Check the scope!

var str = "Learning to Code Javascript Rocks!";
var button = document.getElementById("button");
var output = document.getElementById("output");


output.innerHTML = str;

button.addEventListener("click", function(e){
	str = str.charAt(str.length - 1) + str.substring(0, str.length - 1);
  output.innerHTML = str;
});
button{
    display: block;
    margin: 25px 0;
}
<button id="button">Click Me!</button>

<label id="output"></label>

share|improve this answer
    
This answer is much better than mine. Take an upvote. I was too lazy to actually do this part of it. Very nice. – thesublimeobject 1 hour ago
    
Thanks, I appreciate it! – Gacci 1 hour ago

It is, in fact, a scoping issue. Your counter in inside the function, so each time the function is called, it gets set to 0. If you want a counter that is outside of the scope, and actually keeps a proper count, you will need to abstract it from the function.

If you want to keep it simple, even just moving clickCount above the function should work.

share|improve this answer

I do not know if it's an issue of scope

Yes, it is an issue of scope, more than one actually.

How?

  1. As pointed out by @thesublimeobject, the counter is inside the function and hence gets reinitialized every time a click event occurs.
  2. Even if you put the counter outside the function, you will still face another scope issue. In the else part of the function, you are manipulation a variable (newString) you initialized inside the if snippet. Since, the if snippet didn't run this time, it will throw the error undefined. (again a scope issue)

A fine approach would be:

  1. take the counter and the defaultString outside the function. If the defaultString gets a value dynamically rather than what you showed in your code, extract its value on page load or any other event like change, etc. rather than passing it inside the function.
  2. Do not assign a new string the result of your manipulation. Instead, assign it to defaultString. This way you probably won't need an if-else loop and a newLastChar to take care of newer results.
  3. Manipulate the assignment to the element accordingly.
share|improve this answer

You can use Javascript closure functionality.

var scrollString = (function() {
var defaultString = "Learning to Code Javascript Rocks!";
return function() {
    // convert the string into array, so that you can use the splice method
    defaultString = defaultString.split('');

    // get last element
    var lastElm = defaultString.splice(defaultString.length - 1, defaultString.length)[0];

    // insert last element at start
    defaultString.splice(0, 0, lastElm);

    // again join the string to make it string
    defaultString = defaultString.join('');
    document.getElementById('Result').innerHTML = defaultString;
    return defaultString;
}
})();

Using this you don't need to declare any variable globally, or any counter element.

To understand Javascript Closures, please refer this: http://www.w3schools.com/js/js_function_closures.asp

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.