1
\$\begingroup\$

Exercise Description

Create a stop watch which has the following features:

  • "Start-/Stop" button

  • "Start-/Stop" button can be triggered by pressing the "s"-key.

  • "Record current time" button.

  • Display the recorded times since start.

  • "Record current time" button can be triggered by pressing the "t" key.

  • "Reset time" button which sets the timer counter back to 0 and deletes all time records.

  • "Reset time" button can be triggered by pressing the "r" key.

My Implementation

(function() {
  'use_strict';
  // -- VARIABLE DEFINITIONS ----------------------------------
  let currentTime;
  let timeRunning;
  let pastTimes = ['<span class="subtitle">Past Times</span>'];
  let intervalId;

  let startStop = document.getElementById('start-stop');
  let reset = document.getElementById('reset');
  let recordTime = document.getElementById('record-time');
  let currentTimeDisplay = document.getElementById('current-time-display');
  let pastTimesDisplay = document.getElementById('past-times-display');

  // -- FUNCTION DEFINITIONS ----------------------------------
  function init() {
    if (intervalId) {
      clearInterval(intervalId);
    }

    currentTime = 0;
    timeRunning = false;
    pastTimes.length = 1;
    currentTimeDisplay
      .textContent = currentTime;

    listPastTimes();
  }

  function triggerStartStop() {
    if (!timeRunning) {
      timeRunning = true;

      intervalId = setInterval(() => {
        currentTime += 10; // Increment

        currentTimeDisplay.textContent = // Write
          (currentTime / 1000).toFixed(2);
      }, 10);
    } else {
      clearInterval(intervalId);
      timeRunning = false;
    }
  }

  function listPastTimes() {
    var html = '<ul class="list-past-times">';

    pastTimes.forEach((item) => {
      html += '<li>' + item + '</li>';
    });

    pastTimesDisplay.innerHTML = html + '</ul>';
  }

  function recordCurrentTime() {
    pastTimes.push((currentTime / 1000).toFixed(2));
    listPastTimes();
  }

  // -- ATTACHING EVENT HANDLER ------------------------		
  startStop.addEventListener('click', triggerStartStop);
  reset.addEventListener('click', init);
  recordTime.addEventListener('click', recordCurrentTime);

  startStop.addEventListener('keydown', (event) => {
    if (event.keyCode === 83) { // 83 === 's' in ASCII 
      triggerStartStop();
    }
  });

  reset.addEventListener('keydown', (event) => {
    if (event.keyCode === 82) { // 82 === 'r' in ASCII 
      init();
    }
  });

  recordTime.addEventListener('keydown', (event) => {
    if (event.keyCode === 84) { // 84 === 't' in ASCII 
      recordCurrentTime();
    }
  });

  init();
})();
body {
  font-family: Verdana;
  color: rgba(50, 50, 50, 1.0);
  background-color: rgba(250, 250, 250, 1.0);
}

.main-wrap {
  max-width: 800px;
  margin: 50px auto;
}

.navbar {
  margin: 20px 0;
}

#past-times-display li {
  list-style: none;
}

.subtitle {
  font-weight: 900;
  display: inline-block;
  margin-bottom: 10px;
}

.list-past-times li:nth-of-type(n+2) {
  margin-bottom: 6px;
}
<div class="main-wrap">
  <h1>Stopwatch Demo</h1>
  <section>
    <div id="current-time-display"></div>
  </section>
  <nav class="navbar">
    <button id="start-stop" tabindex="1">Start/Stop</button>
    <button id="reset" tabindex="2">Reset</button>
    <button id="record-time" tabindex="3">Record Time</button>
  </nav>
  <section id="past-times-display"></section>
</div>

Has become quite a lot of code in the end.

  • Any ideas how one could accomplish the task with less code?

  • Are there points which could be improved or solved more elegantly?

  • Is everything done well?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Best Practices

  • Ideally you should separate your logic from your HTML as much as possible.
  • Building a string with += is less efficient than using a buffer. In other words, push each part of the string into an array and then join() the array. When you use += you're loading the entire string into memory every time you add to the string. Here's a re-write of your listPastTimes function.
function listPastTimes() {
  pastTimesDisplay.innerHTML = 
    '<ul class="list-past-times"><li>' +
    pastTimes.join('</li><li>') +
    '</li></ul>';
}

Stuff that's just my opinion

  • I personally would not use the curlies for your if if you're only calling a function. I would put the function on the same line and lose the curlies. Some people are religious about the curlies and it's ok if you are, but in my opinion it's just adding unnecessary lines to the code. I'll defend that to the death. The Apple SSL bug is not a valid argument for curlies because that developer did not put the code on the same line as the if.
  • This is only gonna work on relatively new browsers (maybe that was inended) - I would have used more widely supported methods.
  • I've really don't like the idea of assigning booleans in an if/else. To me, this feels redundant. I would have done triggerStartStop like this:
function triggerStartStop() {
  intervalId = timeRunning ? 
    clearInterval(intervalId) : 
    setInterval(increment, 10) ;
  timeRunning = !timeRunning;
}

function increment() {
  currentTime += 10; // Increment
  currentTimeDisplay.textContent = // Write
    (currentTime / 1000).toFixed(2);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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