Move all actions and filters outside constructor functions

[aheckler]

We have a lot of actions and filters inside constructor functions. Example:

https://github.com/Automattic/WP-Job-Manager/blob/master/includes/class-wp-job-manager-ajax.php#L14

This makes it hard to, say, remove an action and use your own instead. We should move them all out of __construct.

[kraftbj]

My understanding is __construct isn't the issue per se, but adding actions like add_action( 'wp_ajax_nopriv_job_manager_get_listings', array( $this, 'get_listings' ) );

The $this object makes it hard/impossible to access because it isn't a singleton so identifying the exact instance being used... I'm not sure is possible.

[spencerfinnell]

Singleton or converted to static methods.

add_action( 'init', array( __CLASS__, 'add_endpoint') );

can be removed:

remove_action( 'init', array(  'WP_Job_Manager_Ajax', 'add_endpoint') );

But

add_action( 'job_manager_ajax_get_listings', array( $this, 'get_listings' ) );

Cannot.

I don't think its necessary for all hooks but there are definitely some instances where it would be valuable.

[bradyvercher]

Hey guys, I followed the thread from Post Status over and figured I'd chime in since I was part of that discussion. Although I'd recommend avoiding registering hooks in the constructor method, that isn't really the problem here. The issue is that the instance of the AJAX class can't be accessed.

Instead of instantiating the AJAX class at the bottom of the file where it's defined, that could be moved into WP_Job_Manager::__construct() and assigned to a property (like the forms and post types classes already are):

$this->ajax = new WP_Job_Manager_Ajax();

Then external code could remove those hooks like this:

remove_action( 'wp_ajax_nopriv_job_manager_get_listings', array( $GLOBALS['job_manager']->ajax, 'get_listings' ) );

In some cases it does make sense to keep the hook registration private to make the API more stable, but people tend to try working around that with some pretty nasty hacks.

[tripflex]

Ahhh see but it actually is possible, if you know what you're doing :bowtie: 😜

I created a function that allows you to remove actions/filters without having access to the class object ... and it's compatible with WP 4.7+ which changes how global $wp_filter is setup

https://gist.github.com/tripflex/c6518efc1753cf2392559866b4bd1a53

[jom]

Most of our classes have exposed the single instances in __CLASS__::instance(). I'm in favor of moving these outside of __CLASS__::__construct() and/or shifting them to static methods.

This issue will just be for moving outside of the __CLASS__::__construct() and should be converted into a master issue.

For hooks where we think a static method is more suitable, let's open a separate issue and make the case there.