Adding phpbb prefix to functions

[Oleg]

A question arose in https://github.com/phpbb/phpbb3/pull/1144 whether we should add phpbb prefix to functions.

To be honest I rather thought we wanted this all along.

Therefore in that particular PR I added the prefix as the argument list was changed anyway.

[tumba25]

Totally agree.

[Senky]

+1

[DarkBeing]

+1

[MichaelC]

I'm sure there was an RFC for this a while back and it was agreed. https://github.com/imkingdavid/phpbb3/c ... ask/phpbb_ and https://github.com/unknownbliss/phpbb3/ ... ask/phpbb_

[imkingdavid]

For a while there has been a (mabe unwritten, but enforced) rule that any and all new non-member functions (i.e. anything not within a class) should have a phpbb_ prefix. And of course all classes should be prefixed with phpbb_ for autoloading among other reaons. I'm not sure we need to go through all the trouble of renaming all existing functions as was linked to by Michael, however, as that would definitely break compatibility for all MODs that use any native phpBB function.

[MichaelC]

For a while there has been a (mabe unwritten, but enforced) rule that any and all new non-member functions (i.e. anything not within a class) should have a phpbb_ prefix. And of course all classes should be prefixed with phpbb_ for autoloading among other reaons. I'm not sure we need to go through all the trouble of renaming all existing functions as was linked to by Michael, however, as that would definitely break compatibility for all MODs that use any native phpBB function.

We were only renaming functions that were likely to be used by other softwares (very generic terms like add_user()) that phpBB integrated with but we also added wrappers to keep BC.

Although the wrappers were decapreated and therefore would likely be removed completely in future versions.

[EXreaction]

Unless there is a specific incident with naming collision with other popular software, I do not support renaming existing functions.

3.x is not supposed to cause serious issues for backwards compatibility, that is why 3.1 is 3.1 and not 4.0.

[MichaelC]

Unless there is a specific incident with naming collision with other popular software, I do not support renaming existing functions.

3.x is not supposed to cause serious issues for backwards compatibility, that is why 3.1 is 3.1 and not 4.0.

Did you look at the commits I linked to/read my previous post? All the functions had wrappers so no BC was broken.

[EXreaction]

I do not see how renaming the function and then adding a wrapper in the same location is beneficial.

Wrapping functions in if (!function_exists()) also would make debugging significantly more difficult for board administrators. If there would be a conflict it would be very clear what the issue was; however if there was a conflict, but the wrapper was not created because it checks if (!function_exists()) and some old code calls the original function name, very strange or potentially damaging things can happen to the system because unexpected variables will be sent to an unknown function.

At the least, wrappers should be put in a separate file so they can be cleaned up in the future and managed by the board administrator if they must deal with a specific collision incident and should not be wrapped in if (!function_exists()).