Re: [PEPr] +1 for Tools and Utilities::Address_Book
| From: | Firman Wandayandi | Date: | Sat, 04 Dec 2004 20:05:08 +0000 |
| Subject: | Re: [PEPr] +1 for Tools and Utilities::Address_Book | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-34803@lists.php.net to get a copy of this message | ||
Hi Alan, thanks for vote.
Alan Knowles wrote:
Alan Knowles (http://pear.php.net/user/alan_k) has voted +1 on the proposal for Tools and Utilities::Address_Book. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=156 Vote information: http://pear.php.net/pepr/pepr-vote-show.php?id=156&handle=alan_k This vote is conditional. The condition is: * Category: I think "File Formats" is more suitable * Name: Contact_AddressBook ? - more in line with exisiting packages. * Code - safemode checking for fopen seems counter intuative - just remove the @ from fopen, and expose the error if the system is set up wrong - let the sysadmin fix it.. - PHP_Compat dependancy would be better as optional (and only loaded if the version was less that 4.3) - Address_Book_Parser::getFileContents($filename) ?? file_get_contents / or pull in PHP_Compat? - Address_Book_Builder_csv_outlook_express associative array 0 => '...', 1=>'...' is very odd (you dont need the keys?) - The conversion logic is a little odd, rather than picking a neutral format (eg. find an rfc etc.) - it is very confusing using number maps in some places - key maps in others. - There is no clear internal storage format - it looks a bit like there is a numeric array to store/retrieve the data make the code very difficult to follow. - remove/reduce @ usage - especially infront of 'include'. (if this fails it will be near impossible to solve. and is alot easier to isolate the issue if there is an error message.) - pass by ref: I'm not sure you really need to do this everywhere.I has fixed the package, as requested, may u looking for it. Package sources view: http://firman.dotgeek.org/SourceBrowser/index.php?file=%2FContact_AddressBook-0.1.0dev1-fix1.tgz Package download: http://firman.dotgeek.org/downloads/Contact_AddressBook-0.1.0dev1-fix1.tgz Regards, -- Firman