Skip to content
Permalink
Browse files

How about we save 62 bytes? Also ensure that the arguments array give…

…n to fireWith is copied internally.
  • Loading branch information
jaubourg committed Apr 25, 2012
1 parent f307443 commit 97210d4e700aba1b67ac3652303bb685d129aced
Showing with 61 additions and 83 deletions.
  1. +42 −78 src/callbacks.js
  2. +19 −5 test/unit/callbacks.js
@@ -5,12 +5,10 @@ var optionsCache = {};

// Convert String-formatted options into Object-formatted ones and store in cache
function createOptions( options ) {
var object = optionsCache[ options ] = {},
i, length;
options = options.split( /\s+/ );
for ( i = 0, length = options.length; i < length; i++ ) {
object[ options[i] ] = true;
}
var object = optionsCache[ options ] = {};
jQuery.each( options.split( /\s+/ ), function( _, flag ) {
object[ flag ] = true;
});
return object;
}

@@ -47,7 +45,7 @@ jQuery.Callbacks = function( options ) {
var // Actual callback list
list = [],
// Stack of fire calls for repeatable lists
stack = [],
stack = !options.once && [],
// Last fire value (for non-forgettable lists)
memory,
// Flag to know if list was already fired
@@ -60,48 +58,25 @@ jQuery.Callbacks = function( options ) {
firingLength,
// Index of currently firing callback (modified by remove if needed)
firingIndex,
// Add one or several callbacks to the list
add = function( args ) {
var i,
length,
elem,
type,
actual;
for ( i = 0, length = args.length; i < length; i++ ) {
elem = args[ i ];
type = jQuery.type( elem );
if ( type === "array" ) {
// Inspect recursively
add( elem );
} else if ( type === "function" ) {
// Add if not in unique mode and callback is not in
if ( !options.unique || !self.has( elem ) ) {
list.push( elem );
}
}
}
},
// Fire callbacks
fire = function( context, args ) {
args = args || [];
memory = !options.memory || [ context, args ];
fire = function( data ) {

This comment has been minimized.

Copy link
@gibson042

gibson042 Apr 25, 2012

Member

Is this method public-facing? The signature is much easier to optimize, but not as transparent.

This comment has been minimized.

Copy link
@jaubourg

jaubourg Apr 25, 2012

Author Member

Of course not, this is internal only.

memory = !options.memory || data;
fired = true;
firing = true;
firingIndex = firingStart || 0;
firingStart = 0;
firingLength = list.length;
firing = true;
for ( ; list && firingIndex < firingLength; firingIndex++ ) {
if ( list[ firingIndex ].apply( context, args ) === false && options.stopOnFalse ) {
if ( list[ firingIndex ].apply( data[ 0 ], data[ 1 ] ) === false && options.stopOnFalse ) {
memory = true; // Mark as halted
break;
}
}
firing = false;
if ( list ) {
if ( !options.once ) {
if ( stack && stack.length ) {
memory = stack.shift();
self.fireWith( memory[ 0 ], memory[ 1 ] );
if ( stack ) {
if ( stack.length ) {
fire( stack.shift() );
}
} else if ( memory === true ) {
self.disable();
@@ -115,8 +90,18 @@ jQuery.Callbacks = function( options ) {
// Add a callback or a collection of callbacks to the list
add: function() {
if ( list ) {
var length = list.length;
add( arguments );
// First, we save the current length
var start = list.length;
(function add( args ) {
jQuery.each( args, function( type, arg ) {
if ( ( type = jQuery.type(arg) ) === "array" ) {
// Inspect recursively
add( arg );
} else if ( type === "function" && ( !options.unique || !self.has( arg ) ) ) {
list.push( arg );
}
});
})( arguments );
// Do we need to add the callbacks to the
// current firing batch?
if ( firing ) {
@@ -125,55 +110,35 @@ jQuery.Callbacks = function( options ) {
// we should call right away, unless previous
// firing was halted (stopOnFalse)
} else if ( memory && memory !== true ) {
firingStart = length;
fire( memory[ 0 ], memory[ 1 ] );
firingStart = start;
fire( memory );
}
}
return this;
},
// Remove a callback from the list
remove: function() {
if ( list ) {
var args = arguments,
argIndex = 0,
argLength = args.length;
for ( ; argIndex < argLength ; argIndex++ ) {
for ( var i = 0; i < list.length; i++ ) {
if ( args[ argIndex ] === list[ i ] ) {
// Handle firingIndex and firingLength
if ( firing ) {
if ( i <= firingLength ) {
firingLength--;
if ( i <= firingIndex ) {
firingIndex--;
}
}
jQuery.each( arguments, function( index, arg ) {
if ( ( index = jQuery.inArray( arg, list ) ) > -1 ) {
list.splice( index, 1 );
// Handle firing indexes
if ( firing ) {
if ( index <= firingLength ) {
firingLength--;
}
// Remove the element
list.splice( i--, 1 );
// If we have some unicity property then
// we only need to do this once
if ( options.unique ) {
break;
if ( index <= firingIndex ) {
firingIndex--;
}
}
}
}
});
}
return this;
},
// Control if a given callback is in the list
has: function( fn ) {
if ( list ) {
var i = 0,
length = list.length;
for ( ; i < length; i++ ) {
if ( fn === list[ i ] ) {
return true;
}
}
}
return false;
return jQuery.inArray( fn, list ) > -1;
},
// Remove all callbacks from the list
empty: function() {
@@ -203,13 +168,12 @@ jQuery.Callbacks = function( options ) {
},
// Call all callbacks with the given context and arguments
fireWith: function( context, args ) {
if ( stack ) {
args = [ context, jQuery.merge( [], args || [] ) ];
if ( list && ( !fired || stack ) ) {
if ( firing ) {
if ( !options.once ) {
stack.push( [ context, args ] );
}
} else if ( !( options.once && memory ) ) {
fire( context, args );
stack.push( args );
} else {
fire( args );
}
}
return this;
@@ -209,15 +209,29 @@ test( "jQuery.Callbacks( options ) - options are copied", function() {
expect( 1 );

var options = {
memory: true
unique: true
},
cb = jQuery.Callbacks( options );
cb = jQuery.Callbacks( options ),
count = 0,
fn = function() {
ok( !( count++ ), "called once" );
};
options.unique = false;
cb.add( fn, fn );
cb.fire();
});

test( "jQuery.Callbacks.fireWith - arguments are copied", function() {

expect( 1 );

options.memory = false;
var cb = jQuery.Callbacks( "memory" ),
args = [ "hello" ];

cb.fire( "hello" );
cb.fireWith( null, args );
args[ 0 ] = "world";

cb.add(function( hello ) {
strictEqual( hello, "hello", "options are copied internally" );
strictEqual( hello, "hello", "arguments are copied internally" );
});
});

3 comments on commit 97210d4

@gibson042

This comment has been minimized.

Copy link
Member

gibson042 replied Apr 25, 2012

You may get chided for replacing all that inline code with function calls... Callbacks is one of those modules where performance might trump everything else.

@jaubourg

This comment has been minimized.

Copy link
Member Author

jaubourg replied Apr 25, 2012

The most critical parts (initial object creation and executing callback) are untouched (if you except the initial cost of creating the options cache the first time a string is encountered).

I don't even know what got into my mind when I decided not to use any of the jQuery built-in array and iteration methods: having the duplicate code like this all over the place is asking for trouble. Beside, this has good consequences, for instance Callbacks.add accepting array-like objects and inspecting them.

@gibson042

This comment has been minimized.

Copy link
Member

gibson042 replied Apr 26, 2012

👍 for code reuse and 👍 👍 for Callbacks.add accepting arrayish.

Please sign in to comment.
You can’t perform that action at this time.