[PEPr] +1 for Tools and Utilities::Address_Book
| From: | Alan Knowles | Date: | Sat, 04 Dec 2004 02:18:00 +0000 |
| Subject: | [PEPr] +1 for Tools and Utilities::Address_Book | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-34787@lists.php.net to get a copy of this message | ||
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.
--
Sent by PEPr, the automatic proposal system at http://pear.php.net