JavaScript flipping clock

Question

Here are two simple classes - Digit class maintains digit behaviour - primarily flipping animations, and Clock class handles everything that is related to time data.

I'm going to use this code for job application, please tell me if there are any improvements that could be done and what do you think of it.

Here is the link for working demo: https://abm0.github.io/flipping-clock.

class Digit {
  constructor({ selector, value = 0 }) {
    const digitEl = document.querySelector(selector);

    this.flipperEls = digitEl.querySelectorAll('.flipper');
    this.prevDigitEls = digitEl.querySelectorAll('.prev .digit');
    this.nextDigitEls = digitEl.querySelectorAll('.next .digit');

    this.value = value;
    this.prevValue = null;

    this.renderInitialValue();
  }

  setValue(nextValue) {
    this.prevValue = this.value;
    this.value = nextValue;

    if (this.value === this.prevValue) return;

    this.flip();
  }

  renderInitialValue() {
    const {
      prevDigitEls,
      nextDigitEls,
    } = this;

    [...prevDigitEls, ...nextDigitEls].forEach(el => (el.innerHTML = this.value));
  }

  flip() {
    this.nextDigitEls.forEach(el => el.innerHTML = this.value);
    this.flipperEls.forEach(el => el.classList.add('turned'));

    setTimeout(() => {
      this.prevDigitEls.forEach(el => (el.innerHTML = this.value)); 

      this.flipperEls.forEach((el) => {
        el.classList.remove('turned');
      });
    }, 500);
  }
}

class Clock {
  constructor(props) {
    const baseEl = document.querySelector("#clock");
    const currentTime = this.getCurrentTime();

    this.digits = [
      'hours-tens', 
      'hours-ones',
      'minutes-tens',
      'minutes-ones',
      'seconds-tens',
      'seconds-ones'
    ];

    this.buildDigits(currentTime);
  }

  getCurrentTime() {
    const date = new Date();

    const time = {
      hours: date.getHours(),
      minutes: date.getMinutes(),
      seconds: date.getSeconds(),
    };

    this.formatValues(time);

    return time;
  }

  formatValues(time) {
    Object.keys(time).forEach(key => {
      if (key === "ampm") return;

      let value = time[key];

      if (parseInt(value) < 10) {
        time[key] = "0" + value;
      }

      time[key] = time[key].toString();
    });
  }

  getDigitProps(digitName) {
    const type = digitName.split('-')[0];
    const position = digitName.split('-')[1];

    let positionIndex;

    switch (position) {
      case 'tens':
        positionIndex = 0;
        break;

      case 'ones':
        positionIndex = 1;
        break;
    }

    return { type, position, positionIndex };
  }

  buildDigits(time) {
    this.digits.forEach((digitName) => {
      const { type, position, positionIndex } = this.getDigitProps(digitName);

      const selector = `#${type} .${position}-digit`;

      this[digitName] = new Digit({
        selector,
        value: time[type][positionIndex]
      });
    });
  }

  tick() {
    const time = this.getCurrentTime();

    this.digits.forEach((digitName) => {
      const { type, positionIndex } = this.getDigitProps(digitName);

      this[digitName].setValue(time[type][positionIndex]);
    });

  }
}

const clock = new Clock();

setInterval(() => {
  clock.tick();
}, 1000);

Answer 1

Review

If you are after a entry level job then your code shows you can code, which is a good start.

As an emploier I would ask.

1. Why did you choose to use class syntax over more traditional code styles?
2. How long did it take you to write this code?

Bad points

Some points that detract from the code quality

JS

• Clock constructor takes an argument that is not used.constructor(props)
• Unused variable in Clock constructor const baseEl = document.querySelector("#clock");
• Hard coded object is tested for a key it will never have if (key === "ampm") return;
• Clock can only exist as a single instance, but nothing prevents clock being constructed many times.
• Setting element content via innerHTML even though the content has no markup.this.nextDigitEls.forEach(el => el.innerHTML = this.value);
• Redundancy suggests you are not sure about type coercion if (parseInt(value) < 10) {
• You have Not used getters and setters yet have the functions Digit.setValue, Clock.getCurrentTime
• The Clock takes on a Digit s responsibility ? Clock.getDigitProps
• Statement with only two outcomes written using switch not as a ternary in function Clock.getDigitProps
• Lots of redundant code. An example const {prevDigitEls, nextDigitEls } = this; followed by the line [...prevDigitEls, ...nextDigitEls].
• Use event listener to get the flip animation end rather than using setTimeout

CSS & HTML

Its a complete mess, I was going to fix it but it is unusable and needs to be worked from the ground up.

• Bad name for flip animation class turned which you add to an element to start the animation and remove to reset.
• Your code uses ECMAScript6+, yet you include legacy browser CSS, all the legacy CSS is wasted as the clock will not start on those browsers.
• It would be far better to have a container that contains data properties and then build the elements in code. For example HTML only needs <div class="clock" data-type="flip" data-zone="local" data-format="hh:mm:ss"></div> to create the clock

Rewrite

Its just the code.

Personally I would not have written it as follows because the class syntax forces you to break the rules of encapsulation (No privates) and as its only ever to be a single instance Clock which in reality only needs the tick function which could just be a function.

;(() => {
    const digitNames = ['hours-tens', 'hours-ones', 'minutes-tens', 'minutes-ones', 'seconds-tens', 'seconds-ones'];
    const pad0 = num => ("" + num).padStart(2, "0");
    const time = (d = new Date()) => pad0(d.getHours()) + pad0(d.getMinutes()) + pad0(d.getSeconds());
    const elementsText = (elements, text) => { for (const el of elements) { el.textContent = text } }
    class Digit {
        constructor(name, timeString) {
            this.index = digitNames.indexOf(name);
            const [type, position] = name.split("-");
            const container = document.querySelector(`#${type} .${position}-digit`);
            this.flipperEls = container.querySelectorAll('.flipper');
            this.nextDigitEls = container.querySelectorAll('.next .digit');
            this.currentDigitEls = container.querySelectorAll('.prev .digit');
            this.next = this.val = timeString[this.index];
            this.current = this.val;
        }
        set time(timeStr) {
            if (timeStr[this.index] !== this.val) { 
                this.next = this.val = timeStr[this.index];
                this.flippers = "turn";
                setTimeout(() => {
                    this.current = this.val;
                    this.flippers = "turned";
                }, 500);
            }
        }
        set next(value) { elementsText(this.nextDigitEls, value) }            
        set current(value) { elementsText(this.currentDigitEls, value) }            
        set flippers(action) {
            for (const el of this.flipperEls) { 
                if (action === "turn") { el.classList.add('turned') }
                else { el.classList.remove("turned") }
            }
        }
    }
    class Clock {
        constructor() {
            const timeStr = time();
            this.digits = digitNames.map(name => new Digit(name, timeStr));
            (this.tick = this.ticker.bind(this))(); // start the clock
        }
        ticker() {
            const timeStr = time();
            this.digits.forEach(digit => digit.time = timeStr);
            const now = Date.now();
            const nextSecondIn = ((now / 1000 | 0) + 1) * 1000 - now;
            setTimeout(this.tick, nextSecondIn + (nextSecondIn > 500 ? 0 : 1000));
        }
    }
    new Clock();
})();