Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary require_once from codebase #803

Closed
robocoder opened this issue Jun 15, 2009 · 17 comments
Closed

Remove unnecessary require_once from codebase #803

robocoder opened this issue Jun 15, 2009 · 17 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@robocoder
Copy link
Contributor

In #620, we implemented an autoloader. In this ticket, we clean up the code by removing unnecessary require_once statements. This will require some analysis.

  • core/Piwik.php - add this as a special case to the autoloader or rename it to Helper.php (class Piwik_Helper)?
  • some files contain multiple class definitions, e.g., core/Auth.php contains Piwik_Auth and Piwik_Auth_Result
  • ./piwik.php - keep it as fast as possible; autoloading for a cache miss is ok

Don't change:

  • naming of "core" & "plugins" folders
  • FrontController.php and PluginsManager.php - they have their own autoloaders; don't change this
@robocoder
Copy link
Contributor Author

  • obviously, where we require_once a file that doesn't contain a class, keep it, but turn it into an absolute path (i.e., prefix it with PIWIK_INCLUDE_PATH)

@robocoder
Copy link
Contributor Author

@robocoder
Copy link
Contributor Author

(In [1224]) refs #620, refs #803 - refactor autoloader into core/Loader.php (class
Piwik_Loader); remove unnecessary require_once in core/Common.php

@robocoder
Copy link
Contributor Author

(In [1225]) refs #803 - add autoloader to tests

@mattab
Copy link
Member

mattab commented Jun 15, 2009

  • concerning when two classes are in the same file, are you planning to lookup the class name without last part (to look core/Auth.php for the Auth_Result class) or do you plan to create the new file?

@robocoder
Copy link
Contributor Author

My thought is to leave subclassed Exceptions where they are. We can add code to the autoloader to handle this, assuming I got the renaming right.

Something like Auth_Result should be in its own file for speed and consistency. An alternative is to continue to use require_once for these cases.

@robocoder
Copy link
Contributor Author

Actually, Piwik_Auth_Result isn't a good example as it and Piwik_Auth are currently always used in the same context. (core/Access.php, core/Auth.php, plugins/Login/Auth.php)

@robocoder
Copy link
Contributor Author

(In [1228]) refs #803 - auto-discovery algorithm to handle class Piwik, files with
multiple class definitions (e.g., Piwik_Auth_Result), and fix typo
(reference to Piwik_Api_Proxy).

@robocoder
Copy link
Contributor Author

(In [1229]) refs #803 - temp fix: PluginsManager defines functions in global scope

@robocoder
Copy link
Contributor Author

(In [1231]) refs #803 - temp fix; Translate.php contains global scope functions

@robocoder
Copy link
Contributor Author

(In [1232]) refs #803 - Log/Exception contains classes that won't be found by
autoloader's discovery algorithm; FrontController.php and Common.php
use classes with global scope functions (i.e., this needs to be
refactored)

@robocoder
Copy link
Contributor Author

(In [1233]) refs #803 - Auth is loaded by piwik.php?

@robocoder
Copy link
Contributor Author

Ok, files containing global scope functions will continue to be manually included (via require_once).

@robocoder
Copy link
Contributor Author

(In [1248]) refs #803 comment:11 - rename Log Formatter classes for consistency

@mattab
Copy link
Member

mattab commented Jun 25, 2009

increasing priority - now that the loader is in place, maybe we should aim to close the ticket and remove all require_once - vipsoft if you disagree please reset the milestone

@robocoder
Copy link
Contributor Author

(In [1296]) fixes #803 - remove unnecessary require_once from core, plugins, and parts of
libs. (I didn't touch: open-flash-chart, Zend Framework, and PEAR HTML.)

@robocoder
Copy link
Contributor Author

(In [1299]) refs #803, refs #735 - lower default to 200; remove unnecessary require_once; standardize setting of include_path

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

2 participants