I must point out that I can see no usefulness in this code at all. Its acts like a timer with a randomized 16.66...ms error and provides no frame sync at all.
requestAnimationFrame
does not sync with display frames, rather it tells the compositor to hold visual changes made in the rAF
callback (stored in back buf) until safe to move to display buffer (during VSync).
Most browsers these days also do this with setTimeout
and setInterval
Back to the review.
General points
Better to implement DT
via a factory rather than flat code (see rewrite)
Not sure why you define setRTO
and RTOLoop
outside the object's (DT) declaration.
Good to see Object.seal
though my preference is to use Object.freeze
. This forces the exposed object to act more like an interface, allowing only getters and setters to modify state and ensure object integrity.
Avoid indirect calls as they are just a waste of source code characters and CPU cycles. Eg requestAnimationFrame((RAFtimestamp) => DT.RTOloop(RAFtimestamp));
can be requestAnimationFrame(DT.RTOloop);
You are not guarding your object's state.
callback
is not vetted as a function and thus can throw in your code.
start
and duration
are not checked to be numbers and within a valid range.
Always ensure your object has a valid state.
Undefined unwanted behaviour
The is nothing preventing a new call to setRTO
starting a new rAF loop while an existing one is running. This could cause a serious resource drain as more and more loops wait to terminate.
You need to ensure existing loops will terminate when expected, or prevent new loops starting before current loops have ended. Rewrite uses semaphore running
within DT's closure to prevent more than one loop running at a time.
Use modern JS.
At first I thought you were aiming for legacy browser support, however you have modern syntax (arrow function in DT.RTOLoop
and default parameter in DT.setRTO
) which negates all of the old style code, and I question the need to define performance.now
, if the browser supports arrow function, performance.now
is a given.
Modern JS points.
- More arrow functions.
- Object function shorthand declaration.
{ now(){} }
rather than { now: function(){} }
??
rather than ||
. Eg performance = window.performance ?? {}
Rewrite
- Rewrite creates a single instance of
DT
via a IIFE using a factory pattern.
- DT is simplified to one property
setRTO
- The behaviour is vetted against invalid parameters and will do nothing if not given good data.
- Will not start a new timer until existing one (if any) has completed.
const DT = (() => {
const isNum = val => !isNaN(val) && val < Infinity; // Special case Infinity is not a number
var start, duration, callback, running;
function onFrame(time) {
time - start < duration ?
requestAnimationFrame(onFrame) :
(running = false, callback(time, start));
}
return Object.freeze({
setRTO(cb, duration_, start_ = performance.now()) {
if (cb instanceof Function && isNum(duration_) && isNum(start_) && !running) {
callback = cb;
start = start_;
duration = duration_;
running = true;
onFrame(start);
} else {
// todo Fail code here
}
}
});
})();
DT.setRTO((stamp, start) => console.log(stamp - start), 500);
DT.setRTO((stamp, start) => console.log(stamp - start), 100); // this timer will not start