Perhaps split up classes in existing component-class.php to separate files

[DJPaul]

Some of the -class.php files in the components have many classes in them. While historically okay, with all the new development, it feels to me that the class files are getting pretty long, and I think the larger a file is, the harder it is to understand the code.

WordPress core has a nice pattern of using class-whatever.php for separate classes. It keeps thing manageable in terms of removing distractions of other bits of code in the same file, and the class name in the file name makes some IDEs' features of opening a file by name possible (for example, cmd+p in Sublime Text).

I'm suggesting that we adopt a one class per file approach, and re-factor all the existing classes for this. We could either keep the files in the same directory as the rest of the component, or create a new sub-directory (e.g. bp-groups/classes/class-paul.php).

[boonebgorges]

Yes please. I've been meaning to suggest the same thing for years now.

One class per file sounds perfect. Creating a subdirectory classes seems OK to me, but then I think maybe we should drop the class- prefix for the files. I don't feel strongly about this either way.

[DJPaul]

Let's do this in 2.3. It'll give us time to decide how exactly we want to do this.

[DJPaul]

Per above comment, this was discussed at the last dev chat and derailed into a larger discussion about file naming conventions for the entire component. I'd like to limit this particular ticket to splitting up the bp-component-classes.php files. Using Activity as an example:

• bp-activity/bp-activity-classes.php would be split by class into a new bp-activity/classes/ subfolder.
• bp-activity/bp-activity-classes.php would require each of those new class files in the same order that they are defined today (a kind-of autoloader, if you like). This prevents breaking any custom code that is directly including bp-activity-classes.php (that particular concern is out-of-scope of this ticket, but I wanted to briefly mention it).
• For example, we would add bp-activity/classes/class-bp-activity-activity.php, bp-activity/classes/class-bp-activity-query.php, bp-activity/classes/class-bp-activity-feed.php, etc.
• I'm proposing that this change would ONLY affect classes explicitly within the bp-component-classes.php files. Any other classes (for example BP_Activity_Template, which is in bp-activity-template.php would remain unchanged for now. We could in future decide to move those classes out of those other files.

[boonebgorges]

DJPaul - The plan you've laid out here looks great to me. I like the idea of limiting the current changes to what you've suggested here, and leaving other filename/organization issues for future versions.

[djpaul]

In 9485:

Split each component's classes file, and move each individual class into its own file.

While historically manageable, the previous approach of having the majority of each component's classes in the same file is growing unwieldly with each new version of BuddyPress, and causing an avoidable increase in code complexity.

The existing -classes.php files are now a loading wrapper for the components' classes.

This change only affect classes that were explicitly declared within the -classes.php files. Any other classes, for example those in some components' template functions files, remain untouched for now.

Fixes #6083

[r-a-y]

The __DIR__ constant is only available in PHP 5.3+.

Since we brought back PHP 5.2 to our unit test builds in r9504, this is causing an issue with our tests failing in PHP 5.2.

Perhaps switch out __DIR__ with BP_PLUGIN_DIR?

[johnjamesjacoby]

Might this be why our 5.2 tests occasionally fail? Because of use of __DIR__ to set WP_ROOT_DIR in our test suite?

[johnjamesjacoby]

If we use BP_PLUGIN_DIR we'll need to hardcode the component directory paths. We can use dirname( __FILE__ ) in the meantime as a direct replacement for __DIR__.

[boonebgorges]

Replying to johnjamesjacoby:

If we use BP_PLUGIN_DIR we'll need to hardcode the component directory paths. We can use dirname( __FILE__ ) in the meantime as a direct replacement for __DIR__.

+1

[DJPaul]

I had absolutely no idea DIR was a PHP 5.3 thing. Sorry for the breakage.

[djpaul]

In 9514:

Replace use of __DIR__ as it is not supported on PHP 5.2.

Fixes #6083 again.