4
\$\begingroup\$

I made 3 versions of a counting-up timer in JavaScript:


Version #1

All units of time (seconds, minutes, hours, and days) are updated every second, whether they change or not.

Pro: Shorter code.

Con: Worse performance than version #2.

const timeIntervals = [
    ["day", 86400],
    ["hour", 3600],
    ["minute", 60],
    ["second", 1]
];
var counter = 1;
var tick = () => {
    var gucci = counter;
    for (unit of timeIntervals) {
        $("#" + unit[0]).html(Math.floor(gucci / unit[1]));
        gucci %= unit[1];
    }
    counter++;
};
var timer = window.setInterval(tick, 1000);

Version #2

Each unit of time (seconds, minutes, hours, and days) is updated only at fixed intervals when they change.

Pro: Better performance than version #1.

Con: Longer code.

const timeIntervals = [
    ["day", 86400],
    ["hour", 3600],
    ["minute", 60],
    ["second", 1]
];
var counter = 1;
var tick = () => {
    let zero = false;
    for (unit of timeIntervals) {
        let element = $("#" + unit[0]);
        if (zero) {
            element.html("0");
        }
        else if (counter % unit[1] === 0) {
            element.html(
                parseInt(element.html()) + 1
            );
            zero = true;
        }
    }
    counter++;
};
var timer = window.setInterval(tick, 1000);

Version #3

Each unit of time (seconds, minutes, hours, and days) is incremented up by 1 when the counter before it reaches its maximum limit.

Pro: Doesn't require a counter variable.

Con #1: It will break if the user clicks "Inspect Element" and messes with the HTML.

Con #2: Long code.

const timeIntervals = [
    ["second", 60],
    ["minute", 60],
    ["hour", 24],
    ["day", Infinity]
];
var tick = () => {
    $("#second").html(
        parseInt($("#second").html()) + 1
    );
    for (let i = 0; i < timeIntervals.length; i++) {
        let currentElement = $("#" + timeIntervals[i][0]);
        if (parseInt(currentElement.html()) >= timeIntervals[i][1]) {
            currentElement.html("0");
            let nextElement = $("#" + timeIntervals[i + 1][0]);
            nextElement.html(parseInt(nextElement.html()) + 1);
        }
    }
};
var timer = window.setInterval(tick, 1000);

Which of these 3 codes has the best performance, readability, and structure? Do any of them have security vulnerabilities?

\$\endgroup\$
6
\$\begingroup\$

Performance is not a significant concern, since all three callbacks have very little code, and they execute only once per second. Rather, you should aim for clarity. In my opinion, Version 1 is simplest and easiest to follow.

All three techniques suffer from the same weakness with window.setInterval(…, delay):

delay

The time, in milliseconds (thousandths of a second), the timer should delay in between executions of the specified function or code. If this parameter is less than 10, a value of 10 is used. Note that the actual delay may be longer; see Reasons for delays longer than specified in WindowOrWorkerGlobalScope.setTimeout() for examples.

In particular, the interval may be throttled to 10 seconds for long-running scripts in background tabs, or the callback may be delayed if the JavaScript engine is busy executing other tasks. Furthermore, the whole machine might go to sleep.

Rather, you should check the time difference with every tick. (As a side benefit, this should help address your concern about casual Web Inspector tampering.)

Use destructuring assignment to write more meaningful names than unit[0] and unit[1]. Note that in for (unit of timeIntervals), you neglected to localize unit in any way, so it's global.

The tick function should probably be declared as const rather than var. I'd also prefer to use let rather than var, as better software engineering practice.

Also, as better practice, use jquery.text() rather than .html(), if you know that the content is text without HTML markup.

const timeIntervals = [
    ["day", 86400000],  // milliseconds
    ["hour", 3600000],
    ["minute", 60000],
    ["second", 1000]
];
const tick = (start) => () => {
    let elapsed = Date.now() - start;
    for (let [unit, ms] of timeIntervals) {
        $("#" + unit).text(Math.floor(elapsed / ms));
        elapsed %= ms;
    }
};
let timer = window.setInterval(tick(Date.now()), 1000);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script>
<p>
  <span id="day">0</span> days,
  <span id="hour">0</span> hours,
  <span id="minute">0</span> minutes,
  <span id="second">0</span> seconds
</p>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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