Re: Net::LDAP class
| From: | Alan Knowles | Date: | Fri, 25 Jul 2003 14:20:13 +0000 |
| Subject: | Re: Net::LDAP class | ||
| References: | 1 2 3 4 5 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-18699@lists.php.net to get a copy of this message | ||
PEAR CS police here :)
Requires dont require brackets
|require_once ('PEAR.php');
a few of the var's and methods are need phpdoc comments || if (isset($_config['server'])) return $this->bind();| |should read ||if (isset($_config['server'])) { return $this->bind(); } All indentation should be 4 spaces, a few of the methods are missing a bit of depth there You seem to be missing defines for LDAP_OK - which should be NET_LDAP_OK really, although just using true and false may be simpler. If you extend the base class, you can use _Class_Name as a destructor, rather than _done. || Some of the longer lines could be broken up (I think there's a 80char recommending line max) other non=CS stuff. $_config is normally $_options in most pear classes, although making it private is a matter of personal choice. The comment I was making about __get/__set was related to making the Entry object behave a bit like the midgard objects - eg. $entryobject->someval[0] = 'x'; would automatically do $entryobject->attributes['someval'][0] = 'x'; although I've never tested it with arrays (As LDAP tends to use those for attributes). Regards Alan | Tarjei Huse wrote:||require_once'PEAR.php';
Hi, I've tried to clean up the code in regard of Alans comments. Please take a look at the code again and give me comments to what is needed to make it Pear compliant. You'll find it here: cms.polarmedia.no/midgard/Net_LDAP Tarjei On Thu, 2003-07-24 at 04:23, Alan Knowles wrote:In principle +1 on this, however it needs alot of tidying up :) - phpdoc comments - replace 'short' variables with more descriptive ones eg. $_ - CS indentation, bracket placement, method names, control structures without {}, - class names: Net_Ldap, Net_Ldap_Entry, Net_Ldap_Result - I saw one place where '_set_serach()' (eg. a quasi private) was called externally. - odd code bits: "$v"; rather than just $v "unknown host" . $this->_['host'] . " " . "$conn" rather than "unknown host {$this->_['host']} {$conn}" ldap_entry would be nice with overload('Net_Ldap_Entry') function __get($prop,&$ret) { $ret = $this->attributes[$prop]; } function __set($prop,$val,&$ret) { $this->attributes[$prop] = $val; } Regards Alan Tarjei Huse wrote:Hi, The source can now be found here: http://cms.polarmedia.no/midgard/php-ldap/ The ldap.php[s] file is a set of examples that should show usage. The other one is a file containing all the classes. I know it's not completly pear complient yet, but your comments and suggestions would help :-) Tarjei On Wed, 2003-07-23 at 03:08, Alan Knowles wrote:-- Can you help out? Need Consulting Services or Know of a Job? http://www.akbkhome.comCan you the source online? - give us an idea of what it does? Regards Alan Tarjei Huse wrote:Hi, I've written a PHP implementation of Perls Net::LDAP class as I found it much easier to use than the DB::Ldap class (and is is documented). Would you guys be interested in adding it to Pear? Tarjei Mob: 920 63 413