-
Improvement
-
Resolution: Won't Fix
-
Minor
-
3.0.8
-
None
-
N/A
I have been doing a code review of all phpBB include / require files for scoping and some comments to be picked up in sometime.
require vs include. Strictly if failure in inclusion of the module content will result in script failure, then the modules should be REQUIREd not INCLUDEd. phpBB uses a random mix of include, require, include_once and require_once with no consistent logic behind this. E.g. using include when the included functions are essential; not using include once or one-time predicate when there is no clear functional logic for the calling routine to be a one-time execution.
(ii) included/required modules create (global scope) functions, methods, and global side-effects though such assignments as $GLOBAL[...] = ...; any outer variables are local to the scope of the including and such use is to deprecated. There is only ONE example of this deprecated use: functions_privmsgs.php defines two LOCAL variables: $global_privmsgs_rules and $global_rule_conditions which are then used in one of the include paths ucp_pm::main() in the options mode. It would be better to make these true globals $GLOBAL['global_privmsgs_rules'], etc. rather than hidden side effects.
(iii) some classes dynamically include helper functions (e.g. ucp_pm) and sometimes classes, yet the naming conventions for such modules is inconsistent. E.g. some have a functions_ prefix, some don't and none use class_
I am doing a detailed report on this analysis and the rationale behind this. I'll post the link here and on area51 in a couple of days.