Name of new files only containing classes

General discussion of development ideas and the approaches taken in the 3.x branch of phpBB. The next feature release of phpBB 3 will be 3.3/Proteus.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 3.2.x. If you need support for phpBB 3.2.x please visit the 3.2.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Name of new files only containing classes

Post by nickvergessen » Mon Apr 26, 2010 2:39 pm

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.
Member of the Development-TeamNo Support via PM

User avatar
A_Jelly_Doughnut
Registered User
Posts: 1780
Joined: Wed Jun 04, 2003 4:23 pm

Re: Name of new files only containing classes

Post by A_Jelly_Doughnut » Mon Apr 26, 2010 3:02 pm

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
A_Jelly_Doughnut

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: Name of new files only containing classes

Post by igorw » Mon Apr 26, 2010 3:12 pm

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.

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: Name of new files only containing classes

Post by nickvergessen » Mon Apr 26, 2010 4:25 pm

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.
Member of the Development-TeamNo Support via PM

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: Name of new files only containing classes

Post by igorw » Mon Apr 26, 2010 6:55 pm

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.

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Name of new files only containing classes

Post by imkingdavid » Mon Apr 26, 2010 7:34 pm

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. :)
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: Name of new files only containing classes

Post by igorw » Mon Apr 26, 2010 7:56 pm

imkingdavid wrote:@ 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.

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Name of new files only containing classes

Post by imkingdavid » Mon Apr 26, 2010 7:58 pm

eviL3 wrote:
imkingdavid wrote:@ 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. :)
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: Name of new files only containing classes

Post by igorw » Thu May 06, 2010 1:44 pm

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');
Last edited by igorw on Fri May 07, 2010 1:16 pm, edited 1 time in total.

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: Name of new files only containing classes

Post by naderman » Fri May 07, 2010 2:55 am

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.

Post Reply