Re: [PEPr] -1 for Caching::phpfastcache
[email protected] (Khoa Bui) Mon, 5 Aug 2013 16:09:14 -0700 (PDT)
| Newsgroups | php.pear.dev |
|---|---|
| Message-ID | <[email protected]> |
Oh yeah. ThanksĀ Bretrand again for your comments. Now i Have a list to work on it ^_^ Will get back to u guys again, when I am ready again with this one. Next time, I will send you the code first, before I post on pear, Bretrand. Will you look at it next time for me? ________________________________ From: Bertrand Mansion <[email protected]> To: PEAR developer mailinglist <[email protected]> Cc: Bertrand Mansion <[email protected]>; Khoa Bui <[email protected]> Sent: Monday, August 5, 2013 4:51 PM Subject: [PEPr] -1 for Caching::phpfastcache Bertrand Mansion (http://pear.php.net/user/mansion) has voted -1 on the proposal for Caching::phpfastcache. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=692 Vote information: http://pear.php.net/pepr/pepr-vote-show.php?id=692&handle=mansion Comment: I vote -1 because your code is not ready yet, in my opinion. Have a look at : - accessors methods to set/get options in drivers - call_user_func_array instead of eval() - You might use __invoke() instead of your first function - use an autoloader for drivers or you can load all drivers at once instead of hidding require_once in methods, and get rid of isExistingClass... - follow pear coding standards - use an interface for drivers and maybe a common abstract class - spelling mistakes in sqlite driver - isExistingClass ? heard of http://php.net/manual/en/function.class-exists.php - define property and methods scopes explicitly - rename $this->method to $this->driver or storage - don't die(), use Exceptions or trigger_error - have each driver define the options it needs, not the main class - clean up your code at the bottom of the class - You might have to rename your class if you want it to fit PEAR standards (there is already 2 cache packages in pear) I haven't looked at the drivers code, there might be things to fix there too.