Why is include favored over include_once?

[jakobgt]

Hi PHPBB!

First thanks for doing a great job on making an awesome piece of software.

I'm in the process of integrating authentication from a web site into PHPBB3 and during this process I have read quite a few lines of the PHP code. One thing that struck me is that include(…) are preferred over include_once in many places (274 places to be exact). Is there any particular reason that include_once isn't used instead?

One example is on line 862 in /posting.php:

include($phpbb_root_path . 'includes/functions_user.' . $phpEx);

where the user functions are included. This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).

Anyways what I'm asking is basically whether it would be safe for me to change all include(…) to
include_once(….)?

Thanks again for a nice piece of software.

Best regards

Jakob

[imkingdavid]

IIRC, include_once() creates unnecessary overhead that can be avoided by instead using something like:

Code: Select all

if (!function_exists('some_fairly_common_function'))
{
    include('path/to/file');
}

Where function_exists() could also be class_exists() or defined() depending on how you are determining whether the file has been included.

As for your question

This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).

The core code already takes into account what files have been included, so this isn't an issue (if there is some question, we would use some method like I suggested above). For code modifications by users, it is a good idea to by default do what I suggested above just to be safe (especially if two MODs try to include the same file).

EDIT: See kellanved's (he's a former development team member) answer in this topic.

[Oleg]

One example is on line 862 in /posting.php:

include($phpbb_root_path . 'includes/functions_user.' . $phpEx);

where the user functions are included. This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).

If unguarded includes create a problem, we should probably change them to guarded includes.

[Oleg]

Code: Select all

if (!function_exists('some_fairly_common_function'))
 

The function/class that is used should be checked for existence.

[imkingdavid]

The function/class that is used should be checked for existence.

Yes, I wasn't saying to just use any function from the file, but I didn't have a specific example.

[jakobgt]

Thanks for the quick reply!

IIRC, include_once() creates unnecessary overhead that can be avoided by instead using something like:

Code: Select all

if (!function_exists('some_fairly_common_function'))
{
    include('path/to/file');
} 

I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?

Secondly many of the acp_...php files in includes/acp does not guard the inclusion of includes/functions_user.php and hence if one was to include this file logically in the top of the file in an auth_... file, then one gets an "Cannot redeclare .." error.

[imkingdavid]

I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?

From what I can remember, when phpBB 3.0 was released (iirc around late 2007 or early 2008), even though PHP 5 was released, the vast majority of shared hosts (and therefore the vast majority of phpBB users) were still on PHP 4. Since then, yes most have upgraded, but we have not yet updated the code.

Secondly many of the acp_...php files in includes/acp does not guard the inclusion of includes/functions_user.php and hence if one was to include this file logically in the top of the file in an auth_... file, then one gets an "Cannot redeclare .." error.

I agree that that is problematic, if code before a file is included (unguarded in the core) needs to use functions from that file. But I don't know enough about it the difference in overhead between PHP 4 and 5 or between the *_once functions and the *_exists() functions to suggest that we change to the *_once variants.

And until that, if you need to use a function in an include file that is included (unguarded in the core) after you need it, I guess you could submit a bug report asking the include to be changed to a guarded included for that reason.

[jakobgt]

And until that, if you need to use a function in an include file that is included (unguarded in the core) after you need it, I guess you could submit a bug report asking the include to be changed to a guarded included for that reason.

Okay, no worries. I'll just submit a bug report about it. I'll see if I can find as many places of the unguarded include as possible.

Thanks.

[A_Jelly_Doughnut]

I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?

Its irrelevant anyway, because 3.1 and higher require PHP 5.2, and any change of this magnitude would likely not go in the stable branch.

Might be worth discussing moving to include_once/require_once in the appropriate forum. (3.1 or 3,2 discussion_

[bantu]

We can change unguarded includes to guarded includes in phpBB 3.0.x if absolutely necessary. In 3.1.x we have autoloading anyway, so new code will probably be autoloaded instead of manually loaded.