Skip to content
This repository was archived by the owner on Jun 15, 2022. It is now read-only.

Replace autoloader implementation #86

Merged
merged 1 commit into from
Sep 26, 2018

Conversation

schlessera
Copy link
Contributor

The current autoloader implementation uses set_include_path(), which should be avoided.

Since PHP 5.3, there's no valid reason to use set_include_path() in application code anymore, as:

  • it adds processing overhead and additional filesystem checks to all include/require statements that follow it;
  • following from that, it slows down all other autoloaders as well;
  • it can break in environments where the include path is set at the server level and cannot be changed due to security restrictions;
  • spl_autoload_register() can be fed a callback that does the include directly, avoiding any further processing.

This alternative autoloader implementation uses a direct include in the callback that avoids the above drawbacks and is closer to the generally accepted approach that is now used by the PHP community (usually in the form of an autoloader generated by Composer).

The current autoloader implementation uses `set_include_path()`, which should be avoided.

Since PHP 5.3, there's no valid reason to use `set_include_path()` in application code anymore, as:
- it adds processing overhead and additional filesystem checks to _all_ `include`/`require` statements that follow it;
- following from that, it slows down _all_ other autoloaders as well;
- it can break in environments where the include path is set at the server level and cannot be changed due to security restrictions;
- `spl_autoload_register()` can be fed a callback that does the `include` directly, avoiding any further processing.

This alternative autoloader implementation uses a direct `include` in the callback that avoids the above drawbacks and is closer to the generally accepted approach that is now used by the PHP community (usually in the form of an autoloader generated by Composer).
@stevegrunwell stevegrunwell self-requested a review September 26, 2018 13:29
@stevegrunwell stevegrunwell self-assigned this Sep 26, 2018
@stevegrunwell stevegrunwell added this to the Version 1.0.0 milestone Sep 26, 2018
@stevegrunwell stevegrunwell merged commit 38c5faa into liquidweb:develop Sep 26, 2018
@stevegrunwell
Copy link
Contributor

@schlessera This is a great catch, thank you so much!

@schlessera schlessera deleted the alternative-autoloader branch September 26, 2018 13:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants