Name of new files only containing classes

[nickvergessen]

I wonder, whether we should have a special naming for them.
bantu recognized that naming them functions_ is kind of wrong, as most of them don't have normal functions at all.

[A_Jelly_Doughnut]

Yes. This has bothered me for years.

Having said that, in the 3.1 realm, I'm not sure that it would be optimal to have
class fileupload() in includes/functions_upload.php

and
class nicksSuperAwsmFeature in includes/class/nickssuperawsmfeature.php

[igorw]

This would of course be nice for potential autoloading at some point. One problem is existing inconsistencies that would result in broken compatibility. Another main problem I see is naming conventions, specifically underscores. Say you want to autoload the some_feature class. It could be in includes/some/feature.php or includes/some_feature.php. I don't see how that is going to work efficiently with the current naming conventions.

But it would be nice to have some convention, at least for any new classes. There should also be a guideline for singular/plural. I personally would find includes/cron/task/something.php better than includes/cron/tasks/something.php (for class cron_task_something). It would also potentially allow/simplify some reduced factory-type autoloading of certain modules.

[nickvergessen]

files, images, includes and styles are plural
cache, language and store are singular

It's really some kind of odd. I personally would prefer plurals in most cases.

[igorw]

The advantage of singular is that you could do something among the lines of:

Code: Select all

$cron_task = phpbb_load_class('cron/task/some_task');

Which would nicely resolve to cron_task_some_task. The function would include the file and return the instance. It could also be phpbb::load_class(...), if that is being considered.

Just as a side note.

[imkingdavid]

I read somewhere that phpBB was switching to predominantly camel case... wouldn't that mean that class files would have to be like...fileUpload instead of file_upload? EDIT: Actually, I think that was for 4.0, not 3.1, so scratch that. xD

As for singular/plural, I think that language should become languages, because there is a potential for multiple different languages. I can understand cache being singular because there is only one set of cached files, and store just makes more sense to be singular.

@ evil: I have created a class loader method, which I have used in a couple of different projects, and which could be adapted for this situation if needed. Of course if there is a better way of doing this, that's fine, but I figured I'd provide my way so that it didn't have to be done from scratch later on.

[igorw]

@ evil: I have created a class loader method, which I have used in a couple of different projects, and which could be adapted for this situation if needed. Of course if there is a better way of doing this, that's fine, but I figured I'd provide my way so that it didn't have to be done from scratch later on.

That's the basic idea. But this:

Code: Select all

$class_file = $phpbb_root_path . 'includes/' . $class . '.'  . $phpEx;

will not be able to load very many classes.

[imkingdavid]

@ evil: I have created a class loader method, which I have used in a couple of different projects, and which could be adapted for this situation if needed. Of course if there is a better way of doing this, that's fine, but I figured I'd provide my way so that it didn't have to be done from scratch later on.

That's the basic idea. But this:

Code: Select all

$class_file = $phpbb_root_path . 'includes/' . $class . '.'  . $phpEx;

will not be able to load very many classes.

It works for me, since all of my classes are in the same directory in my project. But it's understandable that the method may need to be altered to work optimally with phpBB to allow MODs and stuff to use it with other folders and such.

[igorw]

I'm going to bump this with a proposal. I would advise against changing any existing filenames, but this would be for new class and filenames. This would probably be added to the coding guidelines.

Files in `includes`

A file in the `includes` directory should contain only one class. Helper classes to be used inside that class can be in the same file, but this is discouraged. Interfaces are to be treated the same way.

A class name must be namespaced using underscores. The class should belong to a package, therefore the class name must be phpbb_<package>_<subpackage...>_<class-identifier>. The package is strongly encouraged, the sub-package(s) are optional.

The filename must apply the same namespacing using directories. Therefore the filename must be includes/<package>/<subpackage...>/<class-identifier>.php. The forward slashes `/` represent directories.

An exception is to be made if the class identifier matches the package name. Such as diff/diff. In this case the class name may be shortened to `phpbb_diff`, omitting the package name. The filename however remains includes/diff/diff.php

Example

Class phpbb_cron in includes/cron/cron.php

Class phpbb_cron_task in includes/cron/task.php

Class phpbb_cron_task_base in includes/cron/task/base.php

Class phpbb_cron_task_core_prune_forum in includes/cron/task/core/prune_forum.php

Conflicts

Because class identifiers can contain underscores there is no way to autoload classes. To illustrate this, take the last example. Autoload uses the class name to get the filename for the class. The given name `phpbb_cron_task_core_prune_forum` can be resolved to any of the following:

includes/cron/task/core/prune_forum.php
includes/cron/task/core/prune/forum.php
includes/cron/task/core_prune/forum.php
includes/cron/task_core/prune/forum.php

Therefore the system must know whether it is a namespacing underscore or an underscore within the package/identifier name.

Class loading

To load a class you should use the phpbb_load_class() function. It will take a class name, namespaced using forward slashes `/`. The `phpbb_` prefix must be omitted here. To load the classes from the first example you would use:

$cron = phpbb_load_class('cron/cron');
$cron_task = phpbb_load_class('cron/task');
$cron_task_base = phpbb_load_class('cron/task/base');
$cron_task_core_prune_forum = phpbb_load_class('cron/task/core/prune_forum');

[naderman]

That sounds very much like what I would suggest. However I would like to see all classes prefixed with phpbb_ since we don't have namespaces in 5.2 yet. That should obviously not be reflected in the paths, but the class loader should not load any non phpbb classes. So the class loader can actually be used as an spl autoloader in conjuction with other projects.