Talk:PEAR2 Standards
From PEAR Wiki
Please read the explanation at Pear2 require about which problems PEAR2 is trying to solve and how this shall be done.
About the use of dirname there is another option to define a constant that a package defines if it is not
if (!defined('PEAR2_PATH')) {
define('PEAR2_PATH', dirname(dirname(__FILE));
}
What is LGPL-CC? Do you have a link? CC has several incarnations, and it's not clear which is chosen
Contents |
[edit] __autoload() should not be assumed
This is in response to the "Loading all files at once" and "*_once including of files not allowed" sections (and to a degree some of the sections following."
I think this is a superbly bad idea with really good intentions. First off, require_once and include_once were specifically developed so we wouldn't need to perform class_exists() checks. For those of you who remember the days before these were available, you know what a pain it is to write conditionals at the top of each class file such as are demonstrated here.
The arguments for using these do not sway me. The PHP 5.2.x series has dramatically fixed performance issues with opcode caches when performing conditional use of require/include_once; optimization as an argument for using class_exists tests make no sense. The argument that this is so that phar will work is poor; if this is how phar needs classes to be written in order to work, it should likely be refactored. require_once and include_once are standard PHP; don't rewrite PHP, please.
Why am I so against this? Let's give an example: say I'm writing a new component that has a number of adapters. The most common design patterns to deal with this situation are a Factory that performs Lazy Loading. Traditionally, this would mean that if six adapters exist and I only ever use a single adapter, only one gets loaded into memory and/or my opcode cache. If the proposal is accepted as is, all six are loaded. This seems like a wholesale waste of resources.
Finally, not all of us use __autoload(). __autoload() is an optional tool designed to make things easier for the end-user developer. Some of us, however, like to make sure that all files will be available even if __autoload() is turned off -- sometimes just for correctness, other times out of a desire for performance. __autoload() should not be a requirement of PEAR2, but rather a goal that all classes will work with __autoload(), and still work regardless of whether or not it is on. Using class_exists() hacks and loading all files regardless of being used is coding for tools, not coding for PHP.
-- weierophinney
[edit] leave require_once()s as mandated
I have to agree with MWOP. The memory resources wasted on any sort of modularized package will be huge. For example, MDB2 will have include all installed subpackages by the current logic instead of lazily loading them as/if they are needed.
If phar has issues with the require_onces being used throughout the code, then a build tool needs to be used to remove them. Using a token at the start and end of the required code at the top would make this very easy to do, as would just enforcing no requires except at the top of the file which common practice already. Then a simple regexp to remove all require/require_once lines is all that's needed when building for phar.
-- tswicegood
[edit] require_once
__autoload() is optional in this setup, thats why there is a allfiles.php for each package. In the case of huge packages like MDB2 you could have an allfiles.php for each subpackage.
The goals of the require_once changes are (there may be other things too, this is just what I remember as the big items):
make the use of __autoload possible be opcache friendly make it possible to use PEAR without messing with your include_path make it easy and consistent for new developers to get started using a pear package (just include the packages allfiles.php and your good to go)
and not really a goal but a benefit is it makes it much easier to serialize a PEAR class since its easy to get all the classes you need include before things are unserialized.
[edit] *_once including of files not allowed
This section needs an exception for the PEAR2_Autoload class
[edit] General comments
- Signature: (MF)
[edit] Class-to-file convention
- Why allow exception here?
Isn't this exactly the point to now have to think again how to relate public classes file-wise to each other, to make a clean and clear rule everyone has to follow?
[edit] Loading all files at once
- Why require and not require_once to avoid double inclusion problems?
- Why this file at all?
Shouldn't be all classfiles pre-loadable (without side effect), so can't this be automated? Or tag which files should automatically be included in the allfiles.php? However, PackageName/allfiles.php definitely doesn't fit the Call-to-file convention, is this the exception mentioned? Shouldn't this be made clearer in the first place maybe?
[edit] Loading other classes
- Obviously I'm missing a major point here. I don't understand why classes from the same package should not use require_once on classes they depend.
[edit] Handling dependencies
- This looks horrible, even when being optional. Shouldn't this be something discussed with internals to that require_once throws an exception for that (ok, it would aways by the internal one, like e.g. Exception_RequiredFileNotfound)
(Ok, I now this is probably not going to happen, still this doesn't look right ...)
[edit] RE: require_once
jeichorn -- okay, __autoload() may be conditional, but the alternative provided in the RFC is to load all files, and this is not acceptable. Maybe I'm reading it wrong, but to my eyes, the statement include/require/require_once/include_once is not allowed for loading class files except in the "load all classes for beginners" file' precludes using require_/include_once, and the following requirement for using the if (!class_exists()) { throw exception } construct does as well.
Why is loading all the files unacceptable, either you care about performance and your using an opcode cache (in which case just loading everything if faster then conditional) or you don't care about performance and then how does loading a few extra files matter -jeichorn
jeichorn-- Have you seen the number of files a package like MDB2 has? And if you're only using a single adapter (which is generally the case in production), why on earth would I want to have all the other adapter class files loaded? Even opcode caches have memory limits, and a policy such as this abuses the purpose of such a cache, which is to cache the code being used, not entire libraries. Use the opcode caches effectively by only loading the code that's needed. --weierophinney there is no reason for each subpackage/driver/adapter not to have its own allfiles.php I agree that needs to be updated and made clear in this document -jeichorn
As for the goals you mention,
* PEAR is already __autoload() friendly * require/include_once are already opcode-cache friendly; even when conditionally used, the file is still cached by opcode caches, which is the primary overhead of using them * and PEAR is already easy to use, as long as it is in your include_path -- the configuration of which is something developers should already understand and be able to manipulate
As for serializability of classes, this is something for a build file and/or script to handle, and could easily be introduced into the installer if necessary.
-- weierophinney
[edit] Prefix all classes
please alter this paragraph from
PEAR2_ prefix is required of all class names, package names, however, will not have a PEAR2_ prefix because in most cases, the installation will be like:
pear install pear2/PackageName
to
PEAR2_ prefix is required of all class names.
Example
class PEAR2_Class_Name
Clarification
package names, however, will not have a PEAR2_ prefix because in most cases, the installation will be like:
pear install pear2/PackageName
or something similar
[edit] Loading all files at once
is this possible too?
<?php
// note the lack of dependency on include_path - essential for beginners
if (!class_exists(PEAR2_Exception)) {
class PEAR2_Exception extends Exception {}
}
$dirname = dirname(__FILE__);
require $dirname . '/Base.php';
require $dirname . '/Exception.php';
require $dirname . '/Drivers/Common.php';
require $dirname . '/Drivers/Simple.php';
require $dirname . '/Drivers/Fancy.php';
?>
-- Cybot 04:29, 13 July 2007 (PDT)
[edit] omit closing PHP tag
I think it's recommended to omit the closing PHP tag (?>), because not including it prevents trailing whitespace from being accidentally "injected" into the output - are we going to do the same on PEAR2? --Till 16:34, 18 July 2007 (PDT)
[edit] Exception/Error Handling Guidelines
The long ago approved RFC for error handling should be integrated/referred-to in this document. The last version is here:
http://wiki.ciaweb.net/yawiki/index.php?area=PEAR_Dev&page=RfcExceptionUse
I don't think it differs from the approved RFC:
http://pear.php.net/pepr/pepr-proposal-show.php?id=132
