DevinVinson / WordPress-Plugin-Boilerplate

[WordPress] A foundation for WordPress Plugin Development that aims to provide a clear and consistent guide for building your plugins.
http://wppb.io
7.67k stars 2.25k forks source link

Move class into own file #34

Closed GaryJones closed 11 years ago

GaryJones commented 11 years ago

Good PHP practice says that files should either define stuff (classes, functions and so on) with no side effects, or be a file that uses those definitions and therefore has an effect.

See PSR-1 for an example.

As such, the class itself should be in it's own file. This would, in theory, allow it to be autoloaded or otherwise included by other parts of the system as needed, without it also calling a static method.

tommcfarlin commented 11 years ago

In building pure PHP applications, I agree with this; however, I'm trying to stay consistent with the WordPress Codex article on the Plugin API so that the documentation matches the code that's seen in the boilerplate.

As such, I'm going to leave the class definition and instantiation call at the bottom of the file.

GaryJones commented 11 years ago

There's nothing on the page you linked to that explicitly says that class definitions and the calling of methods from within that class should be in the same file. The demo code shows it right after, but that's more for convenience of showing a code snippet than promoting good practice. If you feel it's wrong, then amend the Codex to indicate which snippet should go in which file.

As per my response in another ticket, don't think of WP development as different to wider PHP application development. Good practices transcend projects.

tommcfarlin commented 11 years ago

To your point, my question then becomes if we were to move the PluginName class into its own file and then place instantiation elsewhere, how do you best propose doing that in such a way that falls inline with the WPCS?

I ask because most documentation I've read shows it done as the boilerplate is doing it, and I've noticed most other plugin developers following the same convention.

Just as you disliked my deviation in terminating code comment blocks because it was a more personal preference (which I was happy to remove - sold on your argument there), I'm not against refactoring this further, but this has typically been the way plugins (and their associated classes) are written and instantiated.

Let me know your thoughts - eager to here 'em.

GaryJones commented 11 years ago

The only impact the WPCS has here, is the name of the class, and the name of the class file. Where that class file is located, or how the contents of it are included is not mentioned IIRC.

(For my plugins which are heavily OOP, I tend to prefer including a PSR-0 compatible autoloader method and naming the class files as per PSR-0 dictates. But for this boilerplate, the class- file name prefix is sufficient.)

I would agree that most plugins are done the way the boilerplate currently is. But most plugins are also written by folks who don't know better (those who stay within what I call the WP bubble). Most plugins also wouldn't split views off into their own file, yet the boilerplate does, because it's a good practice seen in PHP development outside of the WP bubble. Like the views are separated out, because they have a different purpose to other PHP code, so should defined symbols like classes. If you look at a comprehensive plugin like W3TC, then you'll see it keeps everything separate.

Since this project is as much about promoting best practice for new and experienced users, then it should follow those practices, even if the plugin is planned be relatively small, because it allows it to scale easier to something larger later on.

grappler commented 11 years ago

@GaryJones I was looking to add action links to the Plugin Area. I used the code from WooCommerce and when I placed it in class-plugin-name.php it did not work. I then realized it needed to be in the plugin-name.php. After placing it in there it worked. I think it is a bit silly just to have one function in the plugin-name.php.

Is there a better solution with the current file structure?

So you think that the way woocommerce.php is set up is not best practice?

Japh commented 11 years ago

@grappler Untested, but I imagine you just need to change the line: add_filter( 'plugin_action_links_' . plugin_basename( __FILE__ ), array( $this, 'action_links' ) ); to: add_filter( 'plugin_action_links_' . plugin_basename( 'plugin-name.php' ), array( $this, 'action_links' ) );

grappler commented 11 years ago

@Japh Thanks but it did not work.

GaryJones commented 11 years ago

If action_links() was added to the class, then you should need something like the following in your constructor:

$plugin_basename = plugin_basename( plugin_dir_path( __FILE__ ) . 'plugin-name.php' ); // replace "plugin-name"
add_filter( 'plugin_action_links_' . $plugin_basename, array( $this, 'action_links' ) );

If you get it working, it might be a nice thing to do a PR for, so it's a standard function within the boilerplate hint hint :wink:

grappler commented 11 years ago

@GaryJones THANKS!!! :smiley: I have made a PR. https://github.com/tommcfarlin/WordPress-Plugin-Boilerplate/pull/63

GaryJones commented 11 years ago

@grappler You're welcome, and thanks for doing the PR.