Kevin Powell - Community•12mo ago

[Javascript] I did a timer using classes and have some issues

Hey you guys, I'd like to please about rate my code (especially javascript) rate, what should i change, what I did wrong and etc. While it counts font isn't same size all the time, I mean the font slightly moves in different directions (like i screwed up something in css but i don't know) My code is too long so I think i can upload it on pastebin https://pastebin.com/p20rSRKE

Pastebin

My code has some issues - Pastebin.com

Pastebin.com is the number one paste tool since 2002. Pastebin is a website where you can store text online for a set period of time.

2 Replies

Coder_Carl•12mo ago

so a couple of quick fixes 1. dont couple the class to a specific dom node. add an option inside the constructor to feed it in. I also like to make sure that everythings loaded before firing off my functions

      class Timer {
        sec = 0;
        min = 0;
        constructor({ template, elementToDisplayTime }) {
          this.template = template;
          this.elementToDisplayTime = elementToDisplayTime; // we can now use this class anywhere
        }
        timer() {
          this.interval = setInterval(() => {
            if (this.sec < 10) this.sec = "0" + this.sec;
            this.nextFunc();
            this.sec++;
            if (this.sec > 60) {
              this.sec = 0;
              this.min = this.min + 1;
            }
          }, 1000);
        }
        nextFunc() {
          let formattedTime = this.template
            .replace("mm", this.min < 10 ? "0" + this.min : this.min)
            .replace("ss", this.sec);
          // 👇👇👇
          if (this.elementToDisplayTime){
            this.elementToDisplayTime.textContent = formattedTime;
          }
          // 👆👆👆
        }
        stop(){
          clearInterval(this.interval);
        }
      }
 
 
window.addEventListener("load", (event) => {
      const start = document.querySelector(".start");
      const stop = document.querySelector(".stop");
      const displayTime = document.querySelector(".displayTime");
  
      const test = new Timer({
        template: "mm:ss", elementToDisplayTime: displayTime
      });
  
      start.addEventListener("click", () => {
        test.timer();
      });
      stop.addEventListener("click", () => {
        test.stop();
      });
});

      class Timer {
        sec = 0;
        min = 0;
        constructor({ template, elementToDisplayTime }) {
          this.template = template;
          this.elementToDisplayTime = elementToDisplayTime; // we can now use this class anywhere
        }
        timer() {
          this.interval = setInterval(() => {
            if (this.sec < 10) this.sec = "0" + this.sec;
            this.nextFunc();
            this.sec++;
            if (this.sec > 60) {
              this.sec = 0;
              this.min = this.min + 1;
            }
          }, 1000);
        }
        nextFunc() {
          let formattedTime = this.template
            .replace("mm", this.min < 10 ? "0" + this.min : this.min)
            .replace("ss", this.sec);
          // 👇👇👇
          if (this.elementToDisplayTime){
            this.elementToDisplayTime.textContent = formattedTime;
          }
          // 👆👆👆
        }
        stop(){
          clearInterval(this.interval);
        }
      }
 
 
window.addEventListener("load", (event) => {
      const start = document.querySelector(".start");
      const stop = document.querySelector(".stop");
      const displayTime = document.querySelector(".displayTime");
  
      const test = new Timer({
        template: "mm:ss", elementToDisplayTime: displayTime
      });
  
      start.addEventListener("click", () => {
        test.timer();
      });
      stop.addEventListener("click", () => {
        test.stop();
      });
});

2. to stop the jumping that is caused by numbers being slightly different widths we can add some minor styling

.displayTime {
  width: 5ch;
  text-align: left;
}

.displayTime {
  width: 5ch;
  text-align: left;
}

your code is definitely not too long and it fits easily into a codepen https://codepen.io/CA-Carl/pen/dyrvXNG?editors=1111

Joao•12mo ago

Instead of checking manually if the number is less than 10 and adding an additional 0, you can use the padStart function:

let formattedTime = this.template
  .replace("mm", this.min.toString().padStart(2, '0')
  .replace("ss", this.sec.toString().padStart(2, '0');

let formattedTime = this.template
  .replace("mm", this.min.toString().padStart(2, '0')
  .replace("ss", this.sec.toString().padStart(2, '0');

You are also not re-assigning this formattedTime variable anywhere, so use const instead. As a general, rule, declare variables with const first and only use let when you do need to re-assign them. It's slightly more expressive that way. I would also like to see a more consistent naming of methods. For example "timer" to start, why not call it "start"? Also with "nextFunc", doesn't mean anything. It'd be better to use descriptive names so that I don't have to go to the definition of that function to understand what it does. I only want to read that code if I want to understand how it does it.

Gaming

Programming

[Javascript] I did a timer using classes and have some issues