Sérgio Carvalho [2005-09-02 14:20 UTC] Looks cool. My only note is the use of short PHP tags <? in the example:
http://bolk.exler.ru/files/shared/source/example.phps
Evgeny Stepanischev [2005-09-02 15:03 UTC] Oops.. :) Fixed.
Alan Knowles [2005-09-02 15:41 UTC] Looks usefull, and nice and simple to use.
Needs to look at the CS issues - no short if's in shmop.
would be best not to hide errors with @.. - use empty/isset, $php_error etc. or just let the error show if worst case..
Justin Patrin [2005-09-02 17:07 UTC] I don't think System_Shared is th ebest name for this package. System_SharedMemory perhaps?
In Shared.phps please don't use sprintf. It's much slower than normal concatenation and isn't needed here.
Plugin classes should be in the System/SharedMemory/ directory, not Plugins/.
Plugin filenames should be capitalized.
Plugin classes should be named after their file path. System/SharedMemory/File.php should have the class System_Shared_File. Using the same class name is not acceptable.
All plugin classes should inherit from a base class which takes care of the common functions (such as engineName()) and defines the functions to be overridden in the plugins. System_SharedMemory_Common or System_SharedMemory_Plugin would be fine.
@ should not be used in general (it causes errors to happen which the user has no awareness of). Any errors which happen should raise a PEAR_Error instead of being ignored. For example, in the File plugin should the fopen() calls really be silenced and the possibility of an error when opening ignored? Perhaps if the file doesn't exist upon instantiation it should be created.
All ifs (and while, for, do, etc) must have braces.
TRUE, FALSE, and NULL, should be written as true, false, and null.
Evgeny Stepanischev [2005-09-02 17:08 UTC] Okay :) I've fixed it.
Evgeny Stepanischev [2005-09-02 17:18 UTC] Thank you. I'll fix it ASAP
Evgeny Stepanischev [2005-09-02 18:04 UTC] To Justin Patrin:
I've fixed it up.
Johannes Schlüter [2005-09-02 18:43 UTC] It might be nice to get a list of available types from System_SharedMemory, something like a System_SharedMemory::getAvailableTypes() method and using extension_loaded() is imho better than checking a function name in your factory - while this won't work with your file mode.
Evgeny Stepanischev [2005-09-02 18:48 UTC] To Johannes Schlüter:
Yeah, it good idea to add getAvailableTypes to factory class but using extension_loaded is not so good. In safe_mod it may be disabled.
Evgeny Stepanischev [2005-09-02 19:29 UTC] To Johannes Schlüter:
getAvailableTypes - great idea. Added.
Evgeny Stepanischev [2005-09-03 05:39 UTC] Do you need MySQL and/or PDO support?
bertrand Gugger [2005-09-03 10:43 UTC] It's a very valuable idea !
I'm afraid:
return new $class($options);
will throw "Only variable references should be returned by reference" in php4.4, use better:
$ref = & new $class($options);
return $ref;
in getAvailableTypes():
if (function_exists($func) || class_exists($func))...
How can that work ?
Not sure your CVS ID tags are correct
Bon courage !
Evgeny Stepanischev [2005-09-03 10:49 UTC] Thank you! I'll fix ref bug ASAP.
What wrong with "if (function_exists($func) || class_exists($func))"? And what wrong with CVS ID?
Evgeny Stepanischev [2005-09-03 11:58 UTC] bertrand Gugger:
reference bug was fixed.
bertrand Gugger [2005-09-04 11:31 UTC] Not sure you got it , as I'm black listed by your server. (I change IP each day, normally)
> What wrong with "if (function_exists($func) || class_exists($func))"?
It's not a function but a class
The class name is 'System_SharedMemory_Eaccelerator' and not 'eaccelerator' for example
> what wrong with CVS ID?
I thought you need :
// $Id: Exp $
Evgeny Stepanischev [2005-09-04 11:37 UTC] bertrand Gugger:
I didn't catch what wrong with this code or you didn't catch how it works :)
I'll fix CVS ID tag ASAP. Thank you!
bertrand Gugger [2005-09-04 17:25 UTC] > I didn't catch what wrong with this code or you didn't catch how it works
:)
no, I read php
Evgeny Stepanischev [2005-09-04 18:21 UTC] Let's try to understand each other step by step. What's wrong with this code? :)
bertrand Gugger [2005-09-05 09:36 UTC] As you don't believe me, I installed your tgz.
I had to force it:
ERROR: bad md5sum for file /usr/share/pear/System/SharedMemory/File.php
Then I run your test:
PHP Warning: shmop_open(): unable to attach or create shared memory segment in /usr/share/pear/System/SharedMemory/Shmop.php on line 69
Why is it Shmop ?
Because the first test to succeed is with the *function* shmop_open() which exists in PHP
Happy me, I still can read php.
Btw, this warning occurs only the first time you run the test.
bertrand Gugger [2005-09-05 09:50 UTC] OK, sorry, that's working then :) I finally understand the way you do it.
$exist = System_SharedMemory::getAvailableTypes();
var_dump($exist);
array(3) {
[0]=>
string(5) "Shmop"
[1]=>
string(7) "Systemv"
[2]=>
string(4) "File"
}
That's correct for a basic php without extensions.
Evgeny Stepanischev [2005-09-05 12:42 UTC] I've fixed up ID tag, md5 sum and strange shmop_open behavior.
|
