[RFC|Merged] Autoloading & Class Naming Convention

[naderman]

The class naming convention is already documented in coding guidelines.

[igorw]

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).

[naderman]

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.

[Erik Frèrejean]

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

[igorw]

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.

[Erik Frèrejean]

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)

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).

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.

[bantu]

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.

[naderman]

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]

There is a ticket (http://tracker.phpbb.com/browse/PHPBB3-9923) with a proposed patch, objections to it here, and no clear conclusion.

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?

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.

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.

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.

[naderman]

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.