[RFC|Merged] Autoloading & Class Naming Convention

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
User avatar
naderman
Product Manager
Product Manager
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by naderman » Fri Sep 17, 2010 2:33 pm

The class naming convention is already documented in coding guidelines.

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

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by igorw » Fri Sep 17, 2010 4:58 pm

True, but there is no mention of autoloading. A wiki page would be good as it serves as an upgrade path for MOD authors (and also contributors).

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

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by naderman » Fri Sep 17, 2010 8:51 pm

Well yeah that'll be useful for them, but I don't really see the point of anything about that being added to the coding guidelines.

User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by Erik Frèrejean » Tue Nov 23, 2010 2:17 pm

The Ascraeus class loader is very help full although I feel that its way to tightly implemented to the class naming schema. The way it currently is it can only include files from the `phpBB/includes/` directory and when they are prefixed with `phpbb_` this is due to the class naming convention for phpBB. But this poses an problem for MOD authors that want to use the class loader as well but are used to use an MOD specific class prefix `myawesomemod_` or if you develop a larger system that you want to maintain outside the phpBB directory. These MOD authors are forced to either implement their own class loader or don't use it at all.
I'd like to see a much more flexible system implemented where you can add a "load_path" to the class loader where the loader will look if a different prefix than `phpbb_` is used.

proposed: https://github.com/erikfrerejean/phpbb3 ... icket/9923
Available on .com
Support Toolkit developer

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

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by igorw » Tue Nov 23, 2010 3:16 pm

As per the ticket:

Getting rid of the hard-coded 'includes' seems like a good idea. Not so sure about the class prefix, since we really want people to actually namespace their classes. If we want phpbb_mod_classname and /mods/classname.php it might be good to have it dynamic, but that's not been decided yet.

add_loader seems like a bad idea to me, it adds complexity. A better approach here imo would be to just create a second separate class loader instance and call register() on that.

In general YAGNI applies, it does not make sense to change the class loader before we know what the class names and paths of 3.1 MODs will look like. And that discussion probably belongs into a separate topic.

User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by Erik Frèrejean » Wed Nov 24, 2010 2:46 pm

igorw wrote:As per the ticket:

Getting rid of the hard-coded 'includes' seems like a good idea. Not so sure about the class prefix, since we really want people to actually namespace their classes. If we want phpbb_mod_classname and /mods/classname.php it might be good to have it dynamic, but that's not been decided yet.
Well changing it this way you still require people to namespace their classes you just give them the option to chose their own namespace, and using `phpbb_mod_classname_` can still cause conflicts between two MODs. This can be solved by using the `phpbb_mod_modname_classname_` prefix which than points towards `mods/modname/classname.php`, but thats something the MOD author has to do manually. In that case I as an MOD author would rather register my own prefix `modname_` than for every class have to use such an huge classname. Whether this can be done by registering them to the phpBB class loader or using a new instance of the class loader doesn't really matter (although I think the former is more convenient than to have multiple instances of the class loader object all over the place)
igorw wrote:add_loader seems like a bad idea to me, it adds complexity. A better approach here imo would be to just create a second separate class loader instance and call register() on that.
I'm not sure whether I like this, if you for example have 4 MODs installed that all use a different class loader you'll end up with 5 instances of the class loader. IMHO this clutters the code more than needed as the class loader can easily accommodate variations (different include path/class prefix).
igorw wrote:In general YAGNI applies, it does not make sense to change the class loader before we know what the class names and paths of 3.1 MODs will look like. And that discussion probably belongs into a separate topic.
Than I think its time to decide that, as one of the goals of 3.1 is to make modding easier.. I assumed that this was set in stone as the class loader is already merged into develop.
Available on .com
Support Toolkit developer

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by bantu » Thu Dec 09, 2010 12:59 am

igorw wrote:If we want phpbb_mod_classname and /mods/classname.php it might be good to have it dynamic, but that's not been decided yet.
I don't think we should make an expection for the phpbb_mod_ prefix, but rather just call the directory "mod" instead of "mods". This is much more straight foreward.

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

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by naderman » Fri Jan 07, 2011 2:15 pm

In my opinion MODs should simply name their classes phpbb_<modname>_whatever. These files should be placed in the includes directory where all autoloaded classes reside. If we match on non-phpbb_ prefixed classes it becomes difficult to handle dependencies on other libraries that have their own autoloader. If you wish to actually add a project outside of the includes directory, you will have to register your own spl class loader.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Autoloading & Class Naming Convention

Post by Oleg » Sat Apr 30, 2011 8:20 pm

There is a ticket (http://tracker.phpbb.com/browse/PHPBB3-9923) with a proposed patch, objections to it here, and no clear conclusion.
naderman wrote:In my opinion MODs should simply name their classes phpbb_<modname>_whatever. These files should be placed in the includes directory where all autoloaded classes reside.
This is a valid workaround, does everyone agree?
bantu wrote:
igorw wrote:If we want phpbb_mod_classname and /mods/classname.php it might be good to have it dynamic, but that's not been decided yet.
I don't think we should make an expection for the phpbb_mod_ prefix, but rather just call the directory "mod" instead of "mods". This is much more straight foreward.
This I like.
igorw wrote: add_loader seems like a bad idea to me, it adds complexity. A better approach here imo would be to just create a second separate class loader instance and call register() on that.
This I also like. Making "include" into a parameter seems less contentious. How about making "phpbb_" prefix into a parameter? Or a property that can be overridden?

With these changes core autoloader should be usable by just about anything matching our directory structure conventions.
Erik Frèrejean wrote: I'm not sure whether I like this, if you for example have 4 MODs installed that all use a different class loader you'll end up with 5 instances of the class loader. IMHO this clutters the code more than needed as the class loader can easily accommodate variations (different include path/class prefix).
What is the difference between creating another autoloader and registering it vs modifying the core autoloader? It seems to me that it would be negligible. Looking at our autoloader there won't be much duplication of data either.

I would prefer not having code in the core that the core does not need (add_loader and related code) when there is a simple alternative requiring roughly the same amount of effort on the part of modification authors.

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

Re: [RFC|Merged] Autoloading & Class Naming Convention

Post by naderman » Sat Apr 30, 2011 8:24 pm

I agree entirely. MODs should be in includes/mod/<modname> and follow the standard conventions. Making the auto loader generic so you can use another instance for loading from a different location makes sense, but should not be encouraged for regular MODs.

Post Reply