isFunction and object problems

[Andrea Giammarchi]

Version 1.2.6 of jQuery has a public static isFunction method that fails in some case.

These are only two cases:

jQuery.isFunction(new String("function")); // true
jQuery.isFunction({toString:function(){return "function"}}); // true
jQuery.isFunction({valueOf:function(){return "function"}}); // true

At the same time, every compatible browser, IE a part, could simply use this function:

isFunction:function(fn){return fn && fn instanceof Function}

that seems to work as expected with every unit test case.

My suggestion is to implement latter function for Firefox, Opera, and Safari, plus a dedicated one for IE:

isFunction: jQuery.browser.msie ?
        function( fn ) {
            if( !fn )
                return false;
            var s   = "toString",
                v   = "valueOf",
                t   = typeof fn[s] === "function" && fn[s],
                o   = typeof fn[v] === "function" && fn[v],
                r;
            if( t )
                delete fn[s];
            if( o )
                delete fn[v];
            r   = typeof fn !== "string" && !(fn instanceof String) && !fn.nodeName && fn.constructor != Array && /^[\s[]?function/.test(fn + "");
            if( t )
                fn[s]   = t;
            if( o )
                fn[v]   = o;
            return r;
        } :
        function( fn ) {
            return fn && fn instanceof Function;
        },

The only problem is that browser object property is assigned too late, but since it does not depend on jQuery, I suggest to move its assignment on the top of the library, to let other methods use a direct assignment strategy, like above one, instead of a lazy one.

Above code has been tested with every compatible IE, plus Opera 9 and Safari 3 ( I could not test Safari 2 yet )

Kind Regards

[Andrea Giamm]

Just a note, flesler, to respect return boolean value, other browsers could better use this one:

function( fn ) {
    return !!fn && fn instanceof Function;
}

so null or undefined will be returned as false

[Andrea Giamm]

flesler ... I have just realized that first check is not useful ... so this is my last proposal that speedup IE too in an optimistic way:

isFunction: jQuery.browser.msie ?
        function( fn ) {
            if( fn instanceof Function )
                return true;
            if( !fn )
                return false;
            var s   = "toString",
                v   = "valueOf",
                t   = typeof fn[s] === "function" && fn[s],
                o   = typeof fn[v] === "function" && fn[v],
                r;
            if( t )
                delete fn[s];
            if( o )
                delete fn[v];
            r   = typeof fn !== "string" && !(fn instanceof String) && !fn.nodeName && fn.constructor != Array && /^[\s[]?function/.test(fn + "");
            if( t )
                fn[s]   = t;
            if( o )
                fn[v]   = o;
            return r;
        } :
        function( fn ) {
            return fn instanceof Function;
        },

hiping this will help you and will be a "definitive" solution :)

[genezys]

Patch implementing suggested algorithm (simplified and reformatted)

[genezys]

Added test cases for isFunction (merging these one and bug #3134)

[Andrea Giamm]

as I wrote in the ML, please try this code too:

isFunction:function( fn ){
    return !!fn && (fn instanceof Function || (typeof fn == "object" && !(fn instanceof Object)));
}

that with a browser based implementation, to avoid last useless check, could be:

isFunction:jQuery.browser.msie?
function( fn ){
    return !!fn && (fn instanceof Function || (typeof fn == "object" && !(fn instanceof Object)));
}:
function( fn ){
    return fn instanceof Function;
}

Regards

[Andrea Giamm]

Above suggestions fail with different IE cases. Please consider this last piece of code that after some test, seems to be the best one for IE.

isFunction : jQuery.browser.msie ?
function( fn ){
    return  !!fn && (fn instanceof Function || (fn instanceof ActiveXObject && /\[native code\]/.test(fn + "")));
}:
function( fn ){
    return  fn instanceof Function;
}

Hoping this will be the last proposal, I am going to test unit cases.

Kind Regards

[flesler]

By using:

function( fn ) {
    return fn instanceof Function;
}

All tests pass on FF and IE only fails on DOM methods (input.focus, foo.getAttribute). I think this is good enough, it will surely bump perfomance.

I can't imagine a situation where one would separate DOM methods from their node to pass them as argument.

[flesler]

Applied at [5792]. Commented out 2 tests, and the suite passes on all major browsers.

[choan]

Replying to flesler:

By using:

function( fn ) {
    return fn instanceof Function;
}

Unfortunately, memory leaks appear on IE6 when applying instanceof to COM objects. I've created a ticket and provided a patch at #3485

[leidegre]

I've proposed a solution to the built-in DOM function problem here, see  ticket #4536.