2
\$\begingroup\$

I use this Timer function quite a bit lately for all sorts of stuff, and would appreciate if someone could review/analyze/criticize/verify and, of course, suggest things I can do to optimize it for high frequency execution (mainly animation).

My goal was to replicate ActionScript's counterpart (Timer class) and its usage simplicity: registering fns for timer's event dispatch, starting/stopping/resetting the timer, etc.

//
//
;( function ( _host_, _aproto ) {
 var t = true,
 f = false,
 nl = null,
 timerEvent = {
 start : "timer-start",
 timer : "timer",
 stop : "timer-stop",
 end : "timer-end"
 },
 un, // === undefined
 _tm;
 // Array.prototype extensions helpers:
 // .each() .keep() .gc() .has() .empty() .substitute()
 _aproto.each = function ( fn, flgIterBw ) {
 var len = this.length,
 i;
 if ( flgIterBw !== t ) {
 for (
 i = 0;
 i < len;
 i++
 ) {
 if ( fn.call( this, i, this[i] ) === f ) break;
 }
 } else {
 for (
 i = len;
 --i >= 0;
 ) {
 if ( fn.call( this, i, this[i] ) === f ) break;
 }
 }
 return this;
 };
 _aproto.keep = function ( fn ) {
 return this.each(
 function ( i, o ) {
 ( fn.call( this, i, o ) === t )
 || this.splice( i, 1 );
 },
 t
 );
 };
 _aproto.gc = function () {
 var toremove = slc( arguments );
 return this.each(
 function ( i, o ) {
 toremove.has( o )
 && this.splice( i, 1 );
 },
 t
 );
 };
 _aproto.has = function ( v ) {
 return this.indexOf( v ) !== -1;
 };
 _aproto.empty = function () {
 return ( this.length = 0, this );
 };
 _aproto.substitute = function ( arr ) {
 return ( _aproto.push.apply( this.empty(), arr ), this );
 };
 // helper fns
 function isobj( o ) {
 return o === Object( o );
 }
 function isplainobj( o ) {
 return Object.prototype.toString.call( o ) === "[object Object]";
 }
 function isfn( o ) {
 return typeof o === "function";
 }
 function isvalid( o ) {
 return o !== un
 && o !== nl
 && ( o === o );
 }
 function owns( obj, p ) {
 return obj.hasOwnProperty( p );
 }
 // loops objects own properties
 // breaks if fn return false
 function owneach( obj, fn ) {
 if (
 isobj( obj )
 && isfn( fn )
 ) {
 for ( var p in obj ) {
 if ( owns( obj, p ) ) {
 if ( fn.call( obj, p, obj[p] ) === f ) break;
 }
 }
 }
 return obj;
 }
 // attaches set of properties to an object
 function rig_props( obj, props ) {
 if ( isobj( obj )
 && isplainobj( props )
 ) {
 owneach(
 props,
 function ( p, v ) {
 obj[p] = v;
 }
 );
 }
 return obj;
 }
 function slc( arg, i, j ) {
 return Array.prototype.slice.call( arg, i, j );
 }
 function vacate( obj ) {
 for ( var p in obj ) {
 owns( Object.prototype, p )
 || ( delete obj[p] );
 }
 return obj;
 }
 // 'asyncs' a function
 function defer( fn ) {
 var args1 = slc( arguments, 1 );
 return function () {
 var args = args1.concat( slc( arguments ) ),
 target = this,
 origfn = fn;
 setTimeout(
 function () {
 return origfn.apply( target, args );
 }
 );
 return this;
 };
 }
 // gives an object basic event handling support
 // .addListener() .removeListener() .triggerEvent()
 function listener( obj ) {
 if (
 isobj( obj )
 ) {
 var handlers = {};
 rig_props(
 obj,
 {
 // registers set of fns for an event 'e'
 addListener : function ( e ) {
 if (
 isvalid( e )
 ) {
 var fnargs =
 slc( arguments, 1 )
 .keep(
 function ( i, o ) {
 return isfn( o );
 }
 );
 owns( handlers, e )
 && (
 _aproto.push.apply(
 handlers[ e ],
 fnargs
 ),
 t
 )
 || ( handlers[ e ] = slc( fnargs ) );
 }
 return obj;
 },
 // removes fns registered for 'e' event
 removeListener : function ( e ) {
 if (
 isvalid( e )
 ) {
 if ( owns( handlers, e ) ) {
 var fnargs =
 slc( arguments, 1 )
 .keep(
 function ( i, o ) {
 return isfn( o );
 }
 );
 fnargs.length
 && (
 _aproto.gc.apply(
 handlers[ e ],
 fnargs
 ),
 handlers[ e ].length
 || ( delete handlers[ e ] ),
 t
 )
 || (
 handlers[ e ].empty(),
 delete handlers[ e ]
 );
 }
 } else {
 owneach(
 handlers,
 function ( evt, fns ) {
 fns.empty();
 }
 );
 vacate( handlers );
 }
 return obj;
 },
 // runs fns registered for evt 'e'
 triggerEvent : function ( e ) {
 if (
 isvalid( e )
 ) {
 if (
 owns( handlers, e )
 ) {
 var fireargs = slc( arguments, 1 );
 handlers[ e ]
 .each(
 function ( k, evhandler ) {
 defer( evhandler )
 .call(
 obj,
 {
 type : e,
 data : fireargs,
 target : obj,
 handler : evhandler
 }
 );
 }
 );
 }
 }
 return obj;
 }
 }
 );
 }
 return obj;
 }
 //
 // declares Timer factory fn
 _tm = function ( delay, repeatCount ) {
 return ( function ( delay, fireNTimes ) {
 var
 // timer obj
 host = this,
 // timer's private state
 // used/manipulated by api bellow
 timerState = {
 'current-count' : 0,
 'delay' : Math.abs( parseFloat( delay ) ) || 1000,
 'repeat-count' : Math.abs( parseInt( fireNTimes ) ) || Infinity,
 'running' : f,
 'interval' : un
 },
 // arguments provided to timer's .start() method
 // used as args for triggered fns
 fireargs = [];
 // attaches api to timer obj
 // .start() .stop() .reset() .currentCount() .delay() .repeatCount() .running() .state()
 rig_props(
 host,
 {
 // starts timer event dispatch
 // sets provided args as
 // parameters to triggered fns
 // triggers 'timer-start' event
 // and 'timer' events
 start : function () {
 var startargs;
 host.running()
 || (
 timerState.running = t,
 ( startargs = slc( arguments ) ).length
 && fireargs.substitute( startargs ),
 host.triggerEvent.apply(
 host,
 [ timerEvent.start ]
 .concat( fireargs )
 ),
 timerState['current-count'] += 1,
 host.triggerEvent.apply(
 host,
 [ timerEvent.timer ]
 .concat( fireargs )
 ),
 ( timerState['current-count'] === timerState['repeat-count'] )
 && host.reset()
 || ( timerState.interval =
 setInterval(
 function () {
 ( timerState['current-count'] < timerState['repeat-count'] )
 && (
 timerState['current-count'] += 1,
 host.triggerEvent.apply(
 host,
 [ timerEvent.timer ]
 .concat( fireargs )
 ),
 ( timerState['current-count'] === timerState['repeat-count'] )
 && host.reset()
 );
 },
 timerState.delay
 )
 )
 );
 return host;
 },
 // pauses triggering timer events
 // triggers 'timer-stop' event
 stop : function () {
 host.running()
 && (
 ( timerState.interval !== un )
 && (
 clearInterval( timerState.interval ),
 timerState.interval = un
 ),
 timerState.running = f,
 host.triggerEvent.apply(
 host,
 [ timerEvent.stop ]
 .concat( fireargs )
 )
 );
 return host;
 },
 // nulls timer state
 // triggers 'timer-end' event
 reset : function () {
 ( timerState.interval !== un )
 && (
 clearInterval( timerState.interval ),
 timerState.interval = un
 );
 timerState.running = f;
 timerState["current-count"] = 0;
 host.triggerEvent.apply(
 host,
 [ timerEvent.end ]
 .concat( fireargs )
 );
 return host;
 },
 // how many times timer fired
 currentCount : function () {
 return timerState['current-count'];
 },
 // return timer's fire rate in ms
 delay : function () {
 return timerState.delay;
 },
 // how many times timer will fire 'timer' event
 repeatCount : function () {
 return timerState['repeat-count'];
 },
 // returns boolean
 running : function () {
 return timerState.running;
 },
 // returns timers intrnal state{}
 state : function () {
 return {
 currentCount : timerState['current-count'],
 delay : timerState.delay,
 repeatCount : timerState['repeat-count'],
 running : timerState.running
 };
 }
 }
 );
 return host;
 } ).call( listener( {} ), delay, repeatCount );
 };
 //
 // attaches Timer fn to global scope
 _host_.Timer = _tm;
} )( self, Array.prototype );
//
// use:
//
// var
// tm = Timer( 1000/50 ); // set timers fq to 50 times a sec
//
// // register fns for 'timer' event
// tm.addListener(
// "timer",
// function () { console.log( arguments ) },
// doStuff1,
// doStuff2
// );
//
// someElement.onmouseover = function () { tm.start( someElement ); };
// someElement.onmouseout = function () { tm.stop(); };
// someElement.onclick = function () { tm.reset(); };
//
// etc.
//
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 21, 2013 at 0:15
\$\endgroup\$

2 Answers 2

3
+25
\$\begingroup\$

There is room for improvement, in no particular order:

  • Formatting

     for (
     i = 0;
     i < len;
     i++
     ) 
    

    should really follow normal formatting

     for ( i = 0; i < len; i++ )
    
  • Formatting: the splitting of conditionals into separate lines is overdone, function rig_props is the worst case of overdoing it. You can find a fairly authoritative style guide here.

  • Arrow head coding. If code has the following, then it was done wrong:

     }
     );
     }
     );
     }
     }
     return obj;
     }
     }
     );
     }
     return obj;
    }
    
  • Naming: please use camelCasing and meaningful names. The code is too hard to follow (slc, t, nl, un, _tm). I understand you are used to it, but if you ever want other people to understand/maintain this, then you need to fix this.

  • Naming: underscores used to indicate private properties/functions. They seem bad form for parameters (_host_, _aproto). See also the Crockford style guide.

  • It defines .each(), and you should really look into using ForEach() instead. Also, look into reverse() for flgIterBw. forEach can be many times faster than a JS loop.

  • Functions like _aproto.keep should have at least a one-liner comment as to what it does.

  • Come to think of it, the code that enhances the array prototype should really be an object on its own, and not stashed away in Timer as the code is re-usable. This would follow the principle of Separation of Concerns.

  • All in all, the code is hard to maintain, not evenly commented and it does not seem to consider the advances made in JS 1.6 (ForEach, filter, etc.) but it still counts on indexOf().

  • Finally, if the code were to be rewritten with the above in mind, it could get more meaningful code reviews because more reviewers could then grok it.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Sep 24, 2013 at 18:40
\$\endgroup\$
1
\$\begingroup\$

What's with the variable names? len, un, i, fn, t, f? Use the full meaningful names. This is the StopWatch class I use for performance, time measuring.

var StopWatch = function (performance) {
 this.startTime = 0;
 this.stopTime = 0;
 this.running = false;
 this.performance = performance === false ? false : !!window.performance;
};
StopWatch.prototype.currentTime = function () {
 return this.performance ? window.performance.now() : new Date().getTime();
};
StopWatch.prototype.start = function () {
 this.startTime = this.currentTime();
 this.running = true;
};
StopWatch.prototype.stop = function () {
 this.stopTime = this.currentTime();
 this.running = false;
};
StopWatch.prototype.getElapsedMilliseconds = function () {
 if (this.running) {
 this.stopTime = this.currentTime();
 }
 return this.stopTime - this.startTime;
};
StopWatch.prototype.getElapsedSeconds = function () {
 return this.getElapsedMilliseconds() / 1000;
};

Usage

var stopwatch = new StopWatch();
stopwatch.start();
for (var index = 0; index < 100; index++) {
 stopwatch.printElapsed('Instance[' + index + ']');
}
stopwatch.stop();
stopwatch.printElapsed();

Output

Instance[0] [0ms] [0s]
Instance[1] [2.999999967869371ms] [0.002999999967869371s]
Instance[2] [2.999999967869371ms] [0.002999999967869371s]
/* ... */
Instance[99] [10.999999998603016ms] [0.010999999998603016s]
Elapsed: [10.999999998603016ms] [0.010999999998603016s]
answered Sep 22, 2013 at 11:30
\$\endgroup\$
2
  • \$\begingroup\$ The variable names are kind of habit and are enough self explanatory and fast to type in. If that's the single critiq to the code I've pressented, then I'm glad you approve the rest of the code. Btw, that's the slick StopWatch fn you got there, but it's missing a topic's point, where the function registrations part for timer ticks comes in? \$\endgroup\$ Commented Sep 22, 2013 at 13:13
  • \$\begingroup\$ I second the use of descriptive variable names. You're looking to polish your code, not to continue hacking on it, right? The only meaningless variable I use in my code is i, and only when used as a loop index. \$\endgroup\$ Commented Oct 1, 2013 at 15:55

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.