whip icon indicating copy to clipboard operation
whip copied to clipboard

Replace multiple hooks with a static check

Open schlessera opened this issue 7 years ago • 0 comments

Issue https://github.com/Yoast/whip/issues/40 correctly suggested to get rid of the global variable (globals are always bad and should be avoided at all cost).

However, the proposed solution in turns a single check into three function calls that are processed through major parts of the WordPress system (and, ironically, rely on multiple globals as well).

I better fix would have been to turn the global variable into a method-scoped static variable. Serves the same purpose, has no impact outside of the method and does not produce unneeded overhead.

So, something simple like this would have been preferable:

function whip_wp_check_versions( $requirements ) {
    // ...
    static $is_registered = false;
    if ( ! $is_registered ) {
        // Register!
        $is_registered = true;
    }
}

On subsequent calls, the first line is skipped at compile-time already, and the second immediately fails, skipping a duplicate registration. No globals involved (even indirectly through $wp_filters).

schlessera avatar Feb 08 '18 09:02 schlessera