1

My task is to create an HTML button that displays different messages on the page depending on the number of times the button is pressed. My code below contains one global variable (counter) to count the number of times the button is pressed. The code works, but I've read that you should avoid global variables.

var counter = 0; // global variable

window.onload = function() {
  document.getElementById("button").onclick = go;
}

function go() {
  counter++
  switch(counter) {
    case 1:
      document.getElementById("output").textContent = "You pushed the button.";
      break;
    case 2:
      document.getElementById("output").textContent = "You pushed the button (again).";
      break;
    case 3:
    case 4:
    case 5:
      document.getElementById("output").textContent = "You pushed the button "
  + counter.toString() + " times.";
      break;
    default:
      document.getElementById("output").textContent = "Stop pushing the button."
  }
}

I read Define global variable in a JavaScript function which said to wrap the function (go in my case) in a "scoping function" with the global variable directly inside that, like so:

window.onload = function() {
  document.getElementById("button").onclick = go;
}
(function() {
  var counter = 0; // Global now inside the scoping function
  function go() {
    counter++
    switch(counter) {
      case 1:
        document.getElementById("output").textContent = "You pushed the button.";
        break;
      case 2:
        document.getElementById("output").textContent = "You pushed the button (again).";
        break;
      case 3:
      case 4:
      case 5:
        document.getElementById("output").textContent = "You pushed the button "
    + counter.toString() + " times.";
        break;
      default:
        document.getElementById("output").textContent = "Stop pushing the button."
    }
  }
})();

but when I tried that my code didn't work at all. Any suggestions?

Community
  • 1
  • 1
yroc
  • 886
  • 1
  • 10
  • 16
  • 2
    Why did you remove the `window.onload = …` handler in line with wrapping? If you still have it, notice that it references `go`, which is local in that function scope now as well. – Bergi Oct 14 '15 at 16:20
  • 1
    *"didn't work at all"* - Could you explain how? Errors in the console? Note that since `go()` is wrapped inside an anonymous function you your event handler will not be able to see it if you attach it outside that function. – Spencer Wieczorek Oct 14 '15 at 16:21
  • 1
    The `window.onload()` setup code should go inside that anonymous immediately-invoked function. – Pointy Oct 14 '15 at 16:21
  • @Pointy You're right. That fixed it. – yroc Oct 14 '15 at 16:23
  • @Bergi Sorry, that was meant to be a snippet showing only the change, but I see now excluding the window.onload was misleading. – yroc Oct 14 '15 at 16:25
  • @SpencerWieczorek Yes, thank you. And thank you for the suggestion to be more precise in my presentation of the problem :-) – yroc Oct 14 '15 at 16:28

1 Answers1

2

Your window.onload should be inside the scope of your wrapper.

(function() {
  window.onload = function() {
  document.getElementById("button").onclick = go;
  }

  var counter = 0; // Global now inside the scoping function
  function go() {
    counter++
    switch(counter) {
      case 1:
        document.getElementById("output").textContent = "You pushed the button.";
        break;
      case 2:
        document.getElementById("output").textContent = "You pushed the button (again).";
        break;
      case 3:
      case 4:
      case 5:
        document.getElementById("output").textContent = "You pushed the button "
    + counter.toString() + " times.";
        break;
      default:
        document.getElementById("output").textContent = "Stop pushing the button."
    }
  }
})();
Nakib
  • 4,593
  • 7
  • 23
  • 45