Grant loader the power to remove hooks it has added.

[Shelob9]

PR for issue #336

This PR introduces a new method into the Loader called remove. It removes a hook.

In order to do this I have made the following changes besides adding that method:

• I am now indexing the filters& actions properties of the loader class with a hash. Only way I could think of to easily get the right index of array without some sort of stupid foreach or other trickery. At first I just used hook name, but what if two callbacks are hooked to the same hook?
• I made the constructor of the loader class private and introduced a get_instance() method for getting an instance. This annoys me a little, but how else can we make sure that when you remove a hook, you're using the right object of the loader class then using a singleton?
• I used the aforementioned get_instance() method in the main class to set the loader property since if I didn't there would be errors. Errors, as much as I like causing them, are bad.

[slushman]

This is super helpful. I just setup something like this in one my plugins, but I didn't put it into loader. This makes much more sense.

[Shelob9]

BTW My plugin to test this works can be downloaded at https://slack-files.com/T03GVEV0R-F0BN1L9L2-9589568ad1 Look in public class for the hook I added, then conditionally removed using the new remove()

BTW[1] I'm not in love with my use of the singleton here. But, I don't see a better way to ensure we're using the right object when removing. Maybe a helper function, but still...

[DevinVinson]

Thanks for the test plugin. I was just going to try this out.

I'm not sure about the singleton either and really I had been thinking about adding one in the main plugin file to give a single instance of the plugin to work with since I've not found any better option either.

[mAAdhaTTah]

The singleton is really common in WordPress plugins, but I don't think it's necessary for the loader in order to make it possible to remove hooks/filters. If the main class was made a singleton, you could move these static functions there to provide 3rd-party access to your hooks through the plugin's singleton container.

[Shelob9]

I agree and disagree with @mAAdhaTTah

I don't think moving this to the main class, which already has way too many responsibilities. But, if the main class was loaded with a singleton or in some other way (global variable?) its object was accessible later, then we could use $main_class_object->loader->remove(...)

What I like about that is we could then have both a main loader, stored as a property of main class, but if we wanted to, we could create more objects of the loader class if needed. Right now, the private constructor prevents that.

Am I making any sense? If so, I'll refactor this.

[mAAdhaTTah]

If the static methods just provided a "public API" for the loader, then the responsibility of the main plugin is, in addition to loading and containing the plugin's class, to limit access to the plugin's internals except through clearly defined methods. I think we should avoid providing full access to the internals of a plugin by default, and instead allow interaction through public static methods and your standard filters/actions.

That said, I don't know if that's a common pattern or if providing full access to a plugin's internals like in your code snippet is more widespread. If the latter is true, then yeah, @Shelob9's approach makes sense to me.

[Shelob9]

I don't think the new remove method of the loader should be static. It works on a specific object of the loader class, IE the one with the index of hooks, that it added and can therefore remove. Without that, its no better than remove_action/filter

[digitalchild]

Has there been any movement on this? Currently I'm unable to use the standard remove_action to unhook things created in the plugin.

[smileBeda]

This is now possible in the new, “Better WordPress plugin boilerplate” here: https://github.com/TukuToi/better-wp-plugin-boilerplate