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.
//
\$\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.

\$\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]
\$\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

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.