My on/off switch doesn't go off and i don't know how to fix

Question

I need a button that will show and update the time each second since clicked. If it's clicked again it's supposed to show nothing. I tried so much (switch etc.) but i can't solve it. If i click on the button it shows the time and updates it but if i click it again it emptys the innerHTML of the paragraph but a second later the time pops up again and updates itself. Can someone tell me please why does my clearInterval does not work and how i can fix this?

var abc = false;

function switched() {
 let x = document.getElementById("switch");

 if (!abc) { 
  var myFun = setInterval(Time, 1000);
  abc = true;
  return;
 }

 if (abc) {
  clearInterval(myFun);
  abc = false;
  x.innerHTML = "";
  return;
 }

 function Time() {
  var date1 = new Date();
  var t = date1.toLocaleTimeString();
  x.innerHTML = t;
 }
}

<p>On switch which shows you're current time:</p>
<button onclick="switched()">On</button>
<p id="switch"></p>

Don't use 1998 `onclick="switched()"`, get your button on the JS side, e.g using document.querySelector, and then add an event listener with a normal event handler binding (e.g. `btn.addEventListner("click", evt => toggle())`). Also, functions use lowerCamelCase, only classes use UpperCamelCase. And on that note, if you're setting text, don't use `innerHTML`, use `x.textContent = ...`. And finally: good indentation gets you more eyes, always worth auto-formatting your code when putting it in a post. — Mike 'Pomax' Kamermans, Oct 08 '20 at 22:26
The issue is that the `myFun` is scoped too low. It only exists for the duration of the method call. You need to elevate it's scope to the same level as the `abc` — Taplar, Oct 08 '20 at 22:26
It'd really help if you used descriptive names for your variables, instead of `abc`, `x`, `t`, ... — Flater, Oct 08 '20 at 22:31
Welcome! I've modified your code in my answer and provided some pointers and explanation. Use what you know first, and "advance" at your own pace. — iAmOren, Oct 08 '20 at 23:27

iAmOren · Accepted Answer · 2020-10-08T23:43:14.813

Declare the variable holding the interval globally.
Also, the button and the paragraph.
Instead of abc, use the value of the button's textContent.
Call Time right-away to immediately show the time, and then every 1000ms.
Name variables and ids more meaningfully.
Avoid declaring a variable for one-time use.
Describe clearly to the user what is going on and possible actions and results.
Using onclick is perfectly fine. I just recently started using let instead of var, and I guess that here I actually should have used const (I didn't, for debugging), and also, I recently started using textContent instead of innerText = I'm learning too!
(Minor grammatical error: "your", not "you're")

Addendum: Ok, let for interval because it's assigned a value later on (and, most likely, multiple times), and const for the button and paragraph because they will always "point" to the same thing and only that "thing"'s properties will change...

let interval;
const button = document.getElementById("switchButton");
const paragraph = document.getElementById("timeParagraph");

function toggleSwitch() {
  if(button.textContent == "On") {
    button.textContent = "Off";
    Time();
    interval = setInterval(Time, 1000);
  }
  else {
    button.textContent = "On";
    clearInterval(interval);
    paragraph.textContent = "";
  }
}

function Time() {
  paragraph.textContent = new Date().toLocaleTimeString();
}

<p>Turn On to show your current time, Off to clear:</p>
<button id="switchButton" onclick="toggleSwitch()">On</button>
<p id="timeParagraph"></p>

My on/off switch doesn't go off and i don't know how to fix

1 Answers1